Most code review arguments are not about code. They start because the author could not tell whether a comment was a requirement, a preference or a passing thought, and guessed wrong. Say which one it is, aim it at the code rather than the person, and give the reason. Those three habits prevent most of the friction.
Our humor book, HR-Approved Ways to Say What Programmers Are Really Thinking, has a whole section on code review, because every developer has been on both ends of a bad one. This guide is the practical side: how to write the comment you would want to receive.
Say whether the comment blocks the merge
An author who cannot tell which comments are mandatory will either fix everything, which is slow, or argue with everything, which is slower. Google's public code review guidelines recommend labelling the severity of a comment for exactly this reason: without labels, authors may treat every comment as mandatory.
Two common labelling schemes. Google's guidelines suggest a few plain prefixes:
| Label | Meaning |
|---|---|
Nit: | Minor. Technically better, but it won't hugely matter. |
Optional: or Consider: | A good idea, not required. |
FYI: | Not for this change; something to think about later. |
Conventional Comments uses a label plus an optional decoration such as (blocking) or (non-blocking):
| Label | Meaning |
|---|---|
issue (blocking): | A real problem that must be resolved before merging. |
suggestion (non-blocking): | A proposed improvement the author may decline. |
question: | A possible concern you are not sure about. |
praise: | Something done well, said sincerely. |
Sources: Google, How to write code review comments; Conventional Comments.
Whichever scheme your team picks, the rule that matters is honesty: the label has to match the weight. "nit: this whole approach is wrong" is not a nit. As the book puts it, it's a rejection wearing a small hat. If the approach is wrong, say so plainly, early, and with a reason.
Comment on the code, not the coder
"Why did you do it this way?" reads as an accusation even when it is meant as curiosity. The same point aimed at the code reads as engineering:
- Instead of "You didn't handle the empty list." Try "This will raise on an empty list. Can we return early there?"
- Instead of "Why would you use threads here?" Try "The threading here adds complexity, and I can't see a performance gain. Could this stay single-threaded?"
- Instead of "This is confusing." Try "I had to read this twice to follow the retry logic. Would a named helper make it clearer?"
Notice that the rewrites are not softer. They are more specific, which is what makes them easier to act on.
Explain why
A reason turns an order into an argument the author can evaluate, and sometimes win. "Use a set here" invites "why?". "Checking membership in a hash-based set is O(1) on average; in a list it's O(n), and this list can reach thousands of items" answers the question before it is asked, and teaches something the next time it comes up.
Ask questions you actually mean
A real question is one where either answer is acceptable: "Is this endpoint ever called without a user? If not, we can drop the null check." A rhetorical question is a criticism in disguise: "Did you even test this?" Authors can always tell the difference. If you think something is broken, say what you think is broken.
Separate requirements from taste
Thirty comments on naming and formatting, none on behaviour, is the review equivalent of rewriting someone's essay in your own voice. Settle style once, in a linter or formatter the whole team runs, and keep review comments for what a tool can't check: correctness, clarity, edge cases, security and design. If you still want to mention a preference, label it as one.
Review in one pass
Few things sour a review faster than new objections in round five about lines that haven't changed since round one. Read the whole change before commenting, leave everything you have in the first pass, and in later rounds review the changes, not the parts you already accepted. If a change is too large to review in one sitting, the fix is to ask for it to be split, not to review it in instalments.
Say what is good, too
Google's guidelines suggest commenting on things done well, with the reason, and Conventional Comments suggests at least one sincere praise: per review. It is not flattery. It tells the author which habits to keep, and it makes the critical comments read as fair.
When you're the author
- Keep changes small. A "small refactor" that touches 200 files will get a rushed review or no review. Split it.
- Answer every thread before resolving it. Pushing a commit called "fix" and marking a question resolved without replying is how a reviewer learns not to trust your resolutions.
- Say what you tested. "Tested locally" is not evidence. The command you ran and what it showed is.
- Disagree with reasons, too. "I kept the loop because the comprehension was harder to read with three conditions" is a fine answer to a suggestion.
The short version
Label it, aim it at the code, give the reason, and say it all in one pass.
And if you need a laugh after a nine-day wait for a review, the code review section of HR-Approved Ways to Say What Programmers Are Really Thinking was written for you. For the vocabulary behind it (LGTM, nits and the rest), see programmer inside jokes, explained.