Reviewing without starting a fight
Write code review feedback that names a problem, explains its consequence and distinguishes required fixes from suggestions, with a worked example.
Useful code review feedback names a specific behaviour, explains why it matters and says whether a change is required. Review the code against agreed requirements and standards, rather than asking the author to match your personal preferences.
Suppose two requests read a counter at 41 and both save 42. One increase is lost. A useful review comment shows those steps and asks how the update is protected. Calling the code 'bad' gives the author no case to check.
Continue the illustrative counter example above. The reviewer suspects a lost update, but the author says the database already protects it. The next useful step is to inspect that protection together. Repeating that the code is unsafe, or answering that it works, does not settle whether two requests can overwrite one another.
A review comment should make the disagreement checkable. State the behaviour you see, explain the consequence and distinguish a required fix from a question or suggestion. Good wording cannot guarantee agreement, but it can make clear what needs to be resolved.
Agree on what the review must establish#
Google’s review standard favours improvements to the overall health of the codebase without requiring perfection. It also recognises that a well-written feature may still be inappropriate for the system. This is guidance for balancing progress and quality, not permission to ignore a requirement because the rest of the patch looks better.
For the counter, establish what the number represents. A decorative activity count and a count used to allocate the last available seat have different consequences when an update is lost. The requirement determines which behaviour is acceptable; the review then checks whether the implementation provides it.
Keep that decision separate from preferences about naming or helper methods. A reviewer may prefer another design, but should explain what it improves in this change. Where several approaches meet the same requirements and project standards, the author’s choice need not be replaced with the reviewer’s.
Write a comment the author can act on#
In the illustrative example, compare two comments:
| Comment | What the author can do next |
|---|---|
| “You have not thought about concurrency.” | There is no specific execution to inspect, and the comment assumes what the author thought. |
| “If both requests read 41 before either saves, both write 42 and one increase is lost. Is this update protected elsewhere? If not, we need to prevent that before merging.” | Show the existing protection or change the update, then check the overlapping-request case. |
The second comment gives an input, a possible execution and a consequence. It also leaves room for the reviewer to have missed something. Asking about existing protection is a genuine question, not a polite wrapper around a conclusion already treated as certain.
Google’s guidance on review comments recommends discussing the code rather than the developer, explaining the reason for a request and making its importance clear. It allows both suggestions and direct help; reviewers do not have to design every fix themselves.
For this example, a useful response might point to an atomic database update and add a test that would expose a lost increase. Or it might show that the two requests cannot overlap under an enforced constraint. Inspect the actual mechanism rather than accepting “the database handles it” as the explanation. The example does not prescribe a particular database or locking strategy.
Praise can be specific too: “This test sends two overlapping requests and checks that both increases remain. It covers the case we were worried about.” That tells the author which part of the work helped, without promising that one test proves every concurrent execution safe.
Separate required fixes from suggestions#
Use a blocking comment when an unresolved problem must be fixed before this change can be accepted. Name the requirement, risk or agreed standard behind it. For the counter, a lost increase is a blocker if preserving every accepted increase is part of the requirement.
Not every sentence about the system automatically establishes a blocker. “This could be faster” needs a relevant performance requirement or evidence of a problem. “This cannot be rolled back” needs an explanation of the recovery requirement and the consequences of proceeding. The reviewer must connect the concern to this change, not merely name a general engineering virtue.
Mark non-blocking alternatives as optional. For example: “Optional: naming this value previousCount would help distinguish it from the value being saved.” A required naming convention is different from that personal suggestion. Use the team’s agreed style guide rather than inventing a new rule in the review.
Uncertainty also needs a clear label. “Question: can these requests overlap?” is not the same as “Required: prevent a confirmed lost update.” State what observation would resolve the question and whether approval is waiting on it. That is more useful than making the author infer the status from the tone.
Resolve the disagreement and keep the explanation#
A thread stops helping when replies repeat positions without adding evidence or clarifying the requirement. Return to the smallest unresolved question. In the counter example, that might be whether the database operation combines the read and increase, or whether they remain separate operations that can overlap.
A short conversation can help when written discussion stalls. Google’s review standard recommends recording the outcome back in the review and involving an appropriate maintainer or lead when the disagreement remains unresolved. It does not prescribe a universal two-reply limit.
For example, a useful decision note is: “The update already adds one in the database rather than saving the value previously read. We kept that operation and added a test for overlapping requests. The original concern about two writes of 42 does not describe this implementation.” This is an illustrative note, not a report of a test run for this article.
Then decide where the explanation belongs. If the implementation hides an important assumption, make the code clearer or preserve the reason beside it. If the reviewer simply lacked ordinary background knowledge, the reply may be enough. Google’s comment guidance explicitly allows that exception; not every resolved question needs another code comment.
Approval still means accepting responsibility for the review decision. The human merge gate explains what to inspect when machines produce part of the work. When proposals arrive faster than people can check them, the AI vampire and review overload examines the separate workload problem. Better phrasing alone cannot create more review time.
IF YOU REMEMBER ONE THING
Name the behaviour, show why it matters and state what must happen before approval. Keep preferences optional. When the evidence changes your assessment, update the comment and preserve the decision where the next reader needs it.
A review can also expose work that the original estimate omitted. Where estimates go wrong follows the effect of unresolved decisions on the amount of work still to do.
Questions people also ask
4 QUESTIONSShould I approve a change I would have written differently?
A different implementation is not by itself a reason to withhold approval. Check the requirements, relevant risks and agreed project standards. Explain a concrete problem when one exists; otherwise, label your alternative as optional. Google's code-health standard also recognises limits, including a well-written feature that does not belong in the system.
How do I point out a problem without making it personal?
Describe an input, the resulting behaviour and its consequence. For example: 'Two requests can read the same counter value and overwrite one another's increase. Is an atomic update used here?' That is a claim the author can inspect, rather than a judgement about how carefully they worked.
What should a Nit or Optional label mean?
It should tell the author that the suggestion is not a condition of approval. Agree on the team's labels and use them consistently. Do not label a necessary security or correctness fix as optional to soften its wording.
Must every explanation in a review reply become a code comment?
No. Simplify unclear code or preserve a non-obvious reason where future readers need it. A reply can be enough when it explains background knowledge an ordinary maintainer would already have. Do not add comments that merely repeat obvious code.