Code review comments that change things

Most code review comments do not change the code, and of the ones that do, most change something that did not matter.

Here is a taxonomy of comments worth writing, roughly in descending order of value.

the ones worth writing#

"This will break when X." The highest-value comment there is. A specific failure scenario the author did not consider.

"If two requests hit this concurrently, both will pass the existence check and both will insert. We saw this exact bug in the invoicing path last year."

Concrete, falsifiable, and it comes with evidence.

"This contradicts how we do it elsewhere." Consistency has real value and the author frequently does not know the precedent exists.

"orders/ uses the repository pattern for this. Worth matching, or is there a reason to differ here?"

Note the question at the end. Sometimes there is a reason and you have just learned something.

"I don't understand this." Underrated. If a reviewer with context cannot follow it, a stranger in two years will not either.

This is not an admission of inadequacy. It is a measurement of the code's clarity, and it is a measurement only a reader can take.

"What happens if this fails?" The most consistently productive question in code review. Error paths are the least-considered part of most changes and the most-exercised part in production.

"Is this the right layer?" Business logic in a controller, presentation logic in a model, a database call in a template. Structural, cheap to fix now, expensive later.

"This is good and here is why." Genuinely valuable and almost never written. Naming what worked teaches the pattern and it makes the critical comments land better, because they arrive from someone who is paying attention rather than someone who is looking for problems.

the ones not worth writing#

Anything a formatter or linter handles. If you are commenting on spacing, quotes, or import order, fix your tooling instead. A human enforcing mechanical rules is a broken process.

Style preferences without a reason. "I would have used a map here" is not a review comment, it is a preference. If there is a reason — clarity, performance, consistency with a convention — say the reason. If there is not, do not send it.

Speculative generality. "What if we later need to support multiple currencies?" Usually they will not, and building for it costs now. If you genuinely believe it, say what makes you believe it.

A redesign in a review comment. If the approach is fundamentally wrong, that is a conversation, not a comment thread. Comments are for improving an approach; a different approach needs a discussion, and doing it in review comments after the work is done is the most expensive possible time.

That failure is on the process, not the reviewer: the design should have been discussed before implementation.

how to phrase things#

Distinguish blocking from non-blocking. A convention that removes an enormous amount of ambiguity:

blocking: this deletes rows without the tenant filter
suggestion: this could use the existing helper in utils/dates
question: is the retry here intentional given the caller already retries?
nit: typo in the comment
praise: nice — this handles the empty case correctly, which the old one did not

The author knows exactly what to act on. The reviewer can leave a thought without implying it must be addressed. Both people save time.

Ask rather than assert when you are unsure. "Why does this need a lock?" is better than "this does not need a lock" when you are not certain, and it is better even when you are, because the answer might teach you something about the system.

Explain the why, not just the what. "Use a set here" is an instruction. "Use a set here — this is O(n²) on a list that can have thousands of entries" is teaching, and the author will apply it next time without being told.

Comment on the code, not the person. "This is confusing" rather than "you wrote this confusingly." Small difference, and it consistently changes how the comment is received.

the process problems that comments cannot fix#

The diff is too large. Past a few hundred lines, review quality collapses. The comment "this PR is too large, can you split it" is the most valuable one available and it is socially costly to write, which is why nobody does.

Make it a policy so it is not a personal judgment.

Review is too late. If a fundamental problem is found in review, the process failed earlier. That belongs in design.

Only one person reviews. Different reviewers see different things. For anything significant, two, with different backgrounds.

Comments arrive over three days. The author has moved on and has to reload the entire context. Batch your review into one pass, and do it within a day.

the goal#

The purpose of code review is not to find bugs — tests find bugs more reliably and more cheaply.

It is to spread understanding, maintain coherence, and catch the class of problem that automation cannot see: wrong abstraction, wrong layer, missing case, code that will confuse the next reader.

Comments that serve those are worth writing. Everything else is noise with a notification attached.