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

You may not need them but what about others?

Code reviews are tough when people have strong opinions. I try to ask questions rather then making statements, it gives a room for discussion and makes the whole process less challenging

There's definitely a lot to good code reviews. One addition perspective I've started taking that I haven't seen so far in the comments is a "Have you considered this alternative approach type question". And to describe the alternative, show it, etc. And this ensures there is an option to basically reject the suggestion, a valid response is I've considered and rejected that idea or approach.

Is this work you're collaborating on together (as peers) or work you're supervising? Are you in any sort of relationship where you could be considered to be 'mentoring' the developer?

When collaborating, I like to provide my feedback in a way that allows the person to provide their own perspective.

"What do you think about moving this to a separate function so that we can keep each functions in the class small and focused on one specific purpose?"

When supervising you of course want to allow two-way-communication but you can be more direct, but make sure to use the correction as a teaching moment.

"Please move this block to a separate function so that we keep the functions in the class small and focused on their one specific purpose. See abc.js and xyz.js for good examples of classes that follow this pattern."

When you are reviewing, what is your authority level? Do you enforce a checklist? Are you merely asked to give your professional opinion? Are you a senior supervising a junior? These are critical inputs for me to calculate tone.

When I've done reviews, I had some seniority and an organizational privilege to veto some code. I worked from a checklist and a goal (with which the checklist was meant to align, but we knew it was not possible to fully automate those aspects of review). These are my takeaways from that arrangement:

Language like "declined" or "REJECTED" or "can't approve" is discouraging to the individual contributor. I replaced all that with discussion of why I can't allow that, under the obvious subtext of rejection. No need to just rub it in when there is learning to offer.

When indicating required changes, especially where I was more-or-less handing them the replacement code, I always said please. Always.

Most importantly, I gave accurate feedback. I took the time to be sure I was right before I wrote a review. Otherwise what's the point. Even simple patches got tested.