When to request changes in a code review, and when to approve with comments
Othman Shareef · August 25, 2026 · 5 min read · The Craft of Code Review
Every pull request review ends in one of three verdicts, and most reviewers pick between them by mood. Knowing when to request changes in a code review, versus approving with your comments attached, is one of the highest-leverage judgment calls a reviewer makes, because the verdict, not the comments, decides whether the author’s day continues or stalls. The comments carry the information. The verdict carries the cost.
Three verdicts, three signals
- Approve: nothing further needed from me. Merge when ready.
- Approve with comments: I found things worth saying, and I trust you to handle them without another round trip.
- Request changes: merging now would cause concrete harm, and I need to look again before this ships.
These are messages about trust and process, not just quality scores. Google’s review guidance treats review latency as a first-order cost: respond within one business day at the outside, because slow reviews drain velocity and morale together. A request-changes verdict schedules at least one more full latency cycle. That is sometimes exactly right. It should never be an accident.
When to request changes in a code review
The rule that survives contact with real teams: block on correctness, security, or irreversibility, and nothing else. If you cannot finish the sentence “if this merges, X breaks,” “X leaks,” or “X becomes very hard to undo,” you do not have a blocking concern. You have a comment. Irreversibility deserves the emphasis: a badly named local variable is a thirty-second rename next week, while a badly designed public API or database schema accretes dependents the moment it lands. Block on the second kind, comment on the first, and be honest with yourself about which one you are looking at. A useful self-check: if the author shipped it exactly as written and you were on call, would you actually be paged, or merely annoyed? Paged is a block. Annoyed is a comment, however strongly you feel it.
Approve with comments is the default
Everything opinion-shaped lands here: naming, structure you would have chosen differently, style beyond what the linter enforces, the refactor that could ride along but does not have to. Marking severity explicitly makes this verdict work: Conventional Comments prefixes like nit: and suggestion: tell the author what is optional, so an approval with six notes reads as help rather than as a passive-aggressive block. The verdict is also a deal: you extend trust that the author will read and act in good faith, and authors who repeatedly merge without reading the notes are the reason the deal has to stay explicit.
The asymmetric cost of the wrong verdict
Wrongly approving usually costs a bug, a revert, or a follow-up PR: real but bounded, and mostly paid by the code. Wrongly blocking is paid by the person. It costs a latency cycle, teaches the author to route future PRs around you, and over time trains a team to fear review instead of using it. Both failure modes are real, and the asymmetry is not an argument for rubber-stamping: it is an argument for spending your blocks where they are unambiguous. A reviewer who blocks rarely, and only with a nameable harm attached, finds that nobody argues when they do.
Re-review the delta, not the PR
Etiquette after you request changes: when the author pushes fixes, review what changed since your last pass, not the whole PR from scratch. Confirm the blocking issues are resolved, resist discovering brand-new nitpicks on lines you already read, and answer with the same day-one latency you would want yourself. Nothing corrodes a review culture faster than a re-review that arrives three days late and raises objections that were visible in round one.
Say what you reviewed
Every verdict gets sharper with a scope statement: “reviewed the logic and the migration, skimmed the tests, did not look at the Terraform.” It makes an approval honest, tells the author what still needs eyes, and matters most on large PRs where nobody actually reviews everything. The verdict says whether the change can merge. The scope statement says what that verdict is worth. Reviewers who provide both are the ones whose approvals mean something, and whose rare blocks get taken seriously without a fight.