Ask HN: What tone to use in code review suggestions?

We have no guidelines where I work. People have different styles. I think the only one I particularly dislike is

* Please extract this to a separate function.

And I think my favorite is

* Should we extract this to a separate function?

Asking the person for their opinion seems more productive than commanding them to do something. If you word it like a command, then you're basically stating that you know better than the author and the author has underperformed. That's my opinion at least.

Their concern isn't that other reviewers point it out. It's that they think they shouldn't have to follow the guidelines:

> My natural tendency is to do it one way, but our style guidelines prefer it the other way

Which is fine if it's a reason it's an occasionally slip up. But apparently, in spite of knowing this tendency, they frequently submit code to review where they've messed up to such a degree that reading every instance is considered annoying. Think of the poor reviewer who had to put all the comments in!

Fwiw, I used to use respect-by-default until I encountered (multiple times) people who were too emotionally unhealthy and insecure about their abilities to handle it.

I literally had someone write me an impassioned email claiming that questions about why he made a design choice "served only to humiliate him, not an interrogative purpose", and that "when he did something wrong, I should just tell him what to do instead of asking him why he made that choice". This was beyond insane to me: I would hate it if someone spent 15m reviewing a design I had spent days on, and left no space for the possibility that they missed something.

Sometimes there really is a well-established relationship in which both people are aware that one person has much stronger knowledge of how to write healthy code than the other. While my instinct was to still approach the other person as a peer, experience has taught me that it often causes much _more_ friction to maintain this pretense.

I think it depends on how confident you are regarding the change and how important the code is. If something needs to be done, don't use language that suggests it isn't important. Similarly, don't made a big deal out of things that don't matter.

> It's a massive chore to have a teammate that causes stuff like this and I don't envy your teammates or lead (even worse if you're the lead, obviously).

It gets tricky. I haven't had to deal with this friction for many years, but it's not because I entirely avoid the type of directness described in OP's last two examples. It's because I'm careful about who I'm direct with, and because I've ended up in environments that are increasingly devoid of the emotionally-fragile.

I start out with a baseline of assuming that people are as fragile and childish as what your comment alludes to. In many companies, I'm sure this is actually the case. But as I get to know a coworker, it's very often the case that they have the talent, maturity, and emotional stability to handle direct communication (in both directions) without spiraling into an episode.

In my early career, I joined a company with an awful hiring pipeline as the first employee. That was my first introduction to the idea that a lot of people react violently to being treated with respect if they think they don't deserve it. I quickly started treating them like children as you suggest, and the problem immediately vanished.

But as I've progressed in my career, the amount of time I spend around mediocrities has plummeted, and my prior that the person I'm communicating with is a mental adult has risen. I've found that high-productivity professional contexts weed out the emotionally-unstable in the same way they weed out (eg) the disorganized; the value of good-faith efficient communication without mental breakdowns is simply too high.