POST
Code review is a conversation
Aug 20266 MIN READ
There is a kind of code review comment that makes me wince when I read it, and I have written my share of them, so I am not casting stones. It goes like this: "this should be a map instead of a list", or "you should extract this into a helper", or worst of all, a whole block of refactored code pasted into a comment with no explanation. The comment is technically correct. The map is better than the list. And the author is now defensive, because they did not get a reason, they got an order.
Code review is the place where engineering culture is actually made, and most teams treat it as a bug finding exercise. The goal, in that framing, is to catch mistakes before they ship. That is a real goal, but it is the smaller one. The bigger goal is that the people involved get smarter, the codebase gets more consistent, and the next change is easier than the last one. You do not get any of that with a comment that reads like a linter rule with a human voice.
The three audiences of a comment
Every review comment has three audiences, and the mistake is writing for only one. The first audience is the author, who needs to know what to change and why. The second audience is the future reader, who will encounter this code in six months and will benefit from the reasoning being preserved. The third audience is the process itself, because the tone of your comments is teaching the team what reviews are for.
The author only gets the "what" from an imperative comment. They do not get the "why", which is the part that makes them better. They do not get the context that will help the future reader understand why the map is there. When you write "extract this into a helper", you are optimizing for the shortest path to a green review, and you are paying for it with a team that does not know why helpers exist.
Questions beat commands
The most useful comment I ever received was a question. I had written a loop that re-fetched a record inside a loop, a classic N plus one situation, and the reviewer wrote: "does this need to be fresh each iteration, or could you fetch it once?" That question did three things. It flagged the problem, it taught me the pattern, and it respected me enough to let me find the answer. I have never written that pattern again, because I understood it.
A command would have said: "move this fetch outside the loop." I would have fixed it, and I would have learned nothing, because the reason for the fix would have lived only in the reviewer's head.
The question format is not about being polite. It is about transferring the reasoning. "Why is this list ordered by name?" is a better comment than "you should sort this", because it makes the author justify the decision, and either they can (in which case the code should say so) or they cannot (in which case the change is right). The question turns review from a verdict into a conversation, and a conversation is where the learning happens.
Separate the person from the code
This sounds like a cliche, and it is, because it is true and because it is hard. The reason it is hard is that our code feels like an extension of ourselves. A comment on the code is easily read as a comment on us. The reviewer's job is to make that reading harder, by being specific about the code and never making it personal.
Specificity is the trick. "The timeout here could hang forever if the service is slow, consider a deadline" is about the code, and the author can work with it. "This is fragile" is about a judgement with no evidence, and the author has nothing to work with. The best reviewers point at the concrete consequence of the code, not at the character of the author.
The shared standard question
When a review turns into a disagreement, and it will, the useful question is not "who is right". It is "what is the standard here". Teams that argue about style in comments are teams that never wrote down what they value. A short document that says "we prefer pure functions, we prefer explicit data flow, we prefer to fail loudly" converts most style arguments from personal preference into standard conformance.
This is also where the "why" discipline pays off. A codebase that has its reasoning visible, in comments, in commit messages, in review threads, is a codebase where disagreements are resolvable. You can point at the standard, or at the previous decision, or at the measured tradeoff. The alternative is a review thread that is really an argument about taste, and taste arguments never end.
The reviewer is responsible for their own context
A reviewer who comments on every line has either found a genuinely bad PR or is performing. The former is possible but rare. The latter is common and corrosive, because the author learns that every review is a gauntlet and starts writing defensively, hiding decisions instead of explaining them.
The senior move is the opposite: read the whole change first, understand the intent, then comment on the few things that actually matter. If you cannot understand the intent, that is a comment worth making, because it means the change is not communicating itself. A review comment that says "I do not understand why this exists, can you explain in the description" is worth more than a dozen style nits, because it is diagnosing the codebase's communication problem, not the author's grammar.
What the author owes back
Reviews are a two way street, and the author has obligations too. The first is a pull request description that explains the change, not a copy of the diff. The second is context for the decisions: what was considered, what was rejected, why. The third is the willingness to defend those decisions in the thread, and to change them when the reasoning is better.
An author who preemptively explains their tradeoffs changes the review from archaeology into conversation. The reviewer is no longer guessing at intent, they are reacting to it, and the comments get better as a result. The PR description is the first comment in the review thread, and it sets the tone for everything that follows.
The compounding return
A team that reviews as a conversation gets, over time, a codebase where the reasoning is preserved in the threads, a shared sense of what good looks like, and developers who make fewer mistakes because they internalized the patterns instead of just obeying the comments. That last one is the compounding return, and it is the one that is invisible in any single review.
The teams that review as an error report get a different outcome. The same mistakes get caught over and over, because they were corrected, not understood, and nobody got smarter.
Reviews are cheap to write and expensive to get wrong. The sentence "why does this need to be this way" costs nothing and returns an engineer who thinks. The sentence "fix this" costs the same and returns an engineer who obeys. I know which one I want on my team.