A large share of the review comments I have read in my life were not about a problem. They were about a preference, written in the grammar of a correction.
I would have used a map here. I would have pulled this into its own function. I would have named it differently. Each of those might be perfectly good instincts, and none of them tells the author whether anything is actually wrong.
The person receiving them cannot tell the difference. The comments arrive looking equally weighted, so they treat every line as something to fix, and they spend an afternoon rewriting working code into your dialect. What they learn is that passing review means guessing what you personally like. That is not a skill, and it does not transfer to the next reviewer, who likes something else.
So sort your own comments before you post them.
Is it a defect? It is wrong, it will break, it leaks something, it fails on the second call. Say so plainly and say why. This throws when the list is empty and the caller does not catch it.
Is it a rule the team agreed on? Point at the rule rather than at yourself. Our convention is snake case for table names.
Is it your taste? Then label it as taste. Personal preference, take it or leave it. I would probably have used a map here, but this is fine as it stands.
That one word, preference, changes the whole exchange. It gives the author permission to say no without it becoming a negotiation, and it tells them exactly how much weight your opinion is asking for.
Once you start labelling, you will find a surprising share of your comments are taste. That is not a reason to stop making them. Showing someone how you would have approached a problem is genuinely useful, particularly to someone earlier on. It only does damage when it is disguised as a requirement.
And when it is a real defect, do not soften it into a suggestion. Vagueness there is worse than bluntness. The clearer you are about what must change, the freer everyone can be about everything else.
Say what is wrong. Say what is yours. Keep the two apart.
– Asael Shinder
Top comments (0)