Approve with comments is safe for minor, optional or cosmetic notes. It is not safe when the comment describes a vulnerability or a removed control, because an approval tells the merge system the change is ready. Block on security findings and approve once they are fixed.
What the practice is
Google's public guidance on review speed describes "LGTM with comments." A reviewer gives approval while also leaving unresolved comments, in order to avoid making a developer wait a full day for a final "approved." It lists when to do this: the reviewer is confident the developer will appropriately address all remaining comments, or the comments do not have to be addressed, or the suggestions are minor, such as sorting imports, fixing a nearby typo, applying a suggested fix or removing an unused dependency. It adds that the reviewer should say which of these they intend.
It is especially useful when author and reviewer are in different time zones.
Why security findings are different
Every example Google lists is a low risk item. None of them is a vulnerability. That is not an accident. Once you approve, branch protection will usually allow a merge. If the author reads the comment late, misreads it as optional, or the pull request is merged by a bot or another person, the finding ships.
A security finding also tends to need a second look after the fix. "Use the parameterised form" is easy to say and easy to get slightly wrong.
A rule that fits on one line
Approve with comments if the worst outcome of the author ignoring every comment is harmless. Otherwise request changes.
Applied to common comments:
| Comment | Approve with comments? |
|---|---|
| Rename variable, fix typo | Yes |
| Add a log line for diagnostics | Yes |
| Consider tightening this regular expression (hardening) | Yes, if labelled optional |
| This id is not checked against the caller | No, request changes |
| This builds SQL from input | No, request changes |
| This removes the admin check | No, request changes |
| This dependency has a known advisory | Depends on reachability, discuss first |
Make the intent explicit
Google's guidance tells reviewers to state which option they intend when it is not clear. Use labels. In the comment, write "optional" or "nit" for items that can ride along, and "blocking" for anything that cannot. The suggested labels in the same guidance, Nit, Optional or Consider, and FYI, make review intent explicit and help authors prioritise.
Then match the status to the labels. If any comment is blocking, the review status must be request changes. A mismatch, where the status says approved but a comment says blocking, is the situation to avoid.
Time zones without risk
If the reviewer is ending their day and the author is starting theirs, split the review. Approve the safe parts and request changes on the security point alone, and say exactly what would let you re-approve. When the author fixes it, the next reviewer can re-approve within minutes. Waiting a day for one fix is slower than doing it the next morning, but approving a vulnerability is slower still.
Tooling can help
A branch protection rule that dismisses stale approvals when new commits arrive, and a mandatory check that fails on unresolved blocking conversations, make the mismatch harder to create. Both are settings in your hosting platform.
Quick check
Who decides if a comment is blocking?
The reviewer, and they should say so in the comment. If the author disagrees, discuss it, with a security champion or security engineer as tiebreaker.
Can the author mark a security comment resolved?
Preferably the reviewer resolves it after checking the fix. That adds seconds and avoids a finding disappearing by accident.
Sources
- Google Engineering Practices: Speed of code reviews
- Google Engineering Practices: How to write code review comments
Last reviewed: 19 October 2026.
