Communication in Code Review: Comment Tone, Conventional Comments, Disagreement and Review Fatigue
A good review comment addresses the code rather than the person, gives its reason and states clearly whether it blocks the merge; Conventional Comments does this with labels such as issue, suggestion, question and praise, plus blocking and non-blocking decorations. The post rewrites the comments on the coupon pull request in that format and covers how many nits are too many, the author's reply, how to handle disagreement and review fatigue.
Version note: This post is conceptual; it is based on Conventional Comments as published on conventionalcomments.org in November 2026 and on the review guide in Google Engineering Practices. The example comments are about the real coupon pull request from the previous post.
Guess First
In the previous post Ayşe found an N+1 problem in the coupon pull request of Bookstore, the online bookstore used as the shared example across this site's series: while calculating the discount, a separate query went to the database for every book in the cart. Imagine she wrote the same finding in two different ways.
First comment: "Seriously, a query inside a loop? We've talked about this before."
Second comment: "issue (blocking): CalculateDiscountAsync sends a separate query to Books for every line; for a five-book cart the GET request produces eight SQL commands (log attached to the pull request). The category already comes back from GetByIdsAsync; if we add CategoryId to BookInfo, both the query and the direct access to the Catalog's table go away. What do you think?"
Both comments point at the same line. Which one gets fixed faster? Under which one does a three-message argument start? And the harder question: after reading the first comment, will Emre send his next pull request to Ayşe, or look for another reviewer?
Comment on the Code, Not the Person
Review is a written conversation. Face to face, tone of voice and facial expression soften a sentence; in writing there is none of that, and the reader fills the gap with whatever mood they are in that day. That is why a few habits in review comments matter as much as the content.
Talk about the code, not the person. Instead of "you made a mistake here", "this method sends a query on every line". Google's guide recommends the same: rather than asking why the developer did something, describe what the code does and why that is a problem. "Why did you do it this way?" often reads like an accusation in writing.
Give a reason. "Change this" is an order; "change this, because it also runs inside the order transaction" is information. The reason lets the author spot the same mistake on their own next time. If the reason is a document or a measurement, add the link or the output; Ayşe's log showing eight SQL commands turned a debatable opinion into data nobody could argue with.
Ask when you are not sure, but ask a real question. "What happens if the old version runs after the migration?" is a good question, because Ayşe does not fully know the answer either. "You know this is wrong, right?" is criticism dressed as a question.
When suggesting a fix, you can leave the decision to the author. Google's guide says fixing the change is the author's responsibility and the reviewer does not have to design the solution, but should be helpful; sometimes pointing at the problem teaches the author more, sometimes a direct code suggestion saves time.
Say what is good too. Ayşe noting that StringLength(32) in ApplyCouponRequest matches the database column is a small thing, but it tells Emre that this care was noticed and is expected. Praise written with its reason turns into behaviour repeated in the next pull request.
Weak and stronger versions of the same comments in a Turkish-speaking team, LGTM in the last row included:
| Weak | Better |
|---|---|
| This is wrong. | When an amount coupon is larger than the eligible total, the total goes negative; I tried WELCOME25 on a 19.90 cart. |
| Why did you use DateTime.Now? | DateTime.Now returns local time while coupon times are UTC; in Istanbul the coupon looks invalid after 9 pm. Can we use TimeProvider? |
| Rewrite this test. | The test calculates the expected value with the implementation's formula; if the formula is wrong, the test stays green too. Shall we write a hand-calculated value such as 48.90? |
| I wouldn't write it like this. | An alternative: if we move the calculation into a pure function, the test needs no database. The current version works too, your call. |
| LGTM | Approved. I tried the migration on a database with an order from before coupons; the backfill works. |
The last row matters. A one-word approval and an approval that says what was checked are given by pressing the same button, but the second tells the author, and whoever reads the pull request later, what can be trusted.
Conventional Comments: Labelling a Comment's Intent
One of the pitfalls in the previous post was writing an N+1, a stored XSS and a naming suggestion in the same tone: the author cannot tell which one blocks the merge. Conventional Comments, a small standard published by Paul Slaughter under a Creative Commons Attribution 3.0 licence, puts a label stating the intent at the start of every comment:
<label> [decorations]: <subject>
[discussion]
The labels in short, in our own words:
- praise: something you genuinely find good; at least one per review is recommended.
- nitpick: a small request that comes down to taste; by nature it does not block.
- suggestion: a proposed improvement; you say what you suggest and why.
- issue: a specific problem; ideally paired with a suggestion.
- todo: a small but necessary task; unlike a nitpick it is not optional.
- question: a concern you are not sure about; the answer may be "no problem".
- thought: a non-blocking idea that came up during the review.
- chore: process work that must be done before acceptance (for example a changelog entry or documentation).
- note: non-blocking information the reader should know.
A decoration in parentheses can follow the label: (blocking) means the pull request is not accepted until this is resolved, (non-blocking) means it does not block, (if-minor) means do it if the change is small. The specification recommends using decorations sparingly; stacking three or four on one comment hurts readability.
Ayşe's comments on the coupon pull request look like this in that format:
issue (blocking): CalculateDiscountAsync sends a separate query for every line and goes straight to the Catalog's table. For a five-book cart the GET produces eight SQL commands. Let's add CategoryId to BookInfo and calculate the discount from the lines we already have.
issue (blocking): CouponRules did not change in this pull request, but an amount coupon can exceed the eligible total. I added two tests below that turn red; Math.Min looks enough.
issue (blocking): The coupon description uses bypassSecurityTrustHtml, and the description comes from the database. If we use [innerHTML] with the sanitizer, <strong> stays and onerror is dropped. Spec file attached.
issue: The order log contains the customer's e-mail. The customer number is enough to find the order.
suggestion: Let's write the expected values in the coupon tests as hand-calculated numbers; right now the test repeats the formula and its rounding differs from the code.
question (non-blocking): If the old version takes orders for a while during the deploy, Subtotal stays zero. Does that happen in our deploy? If not, one line in the description is enough.
praise: StringLength(32) in ApplyCouponRequest matches the column length, so validation and schema agree. Nice.
nitpick (non-blocking): The normalized variable in FindValidAsync could be normalizedCode, like normalizedEmail in OrderService.
Keeping the labels in English and writing the explanation in Turkish is a deliberate choice in Bookstore's team: the label is a contract that stays fixed so tools and people share the same meaning; the explanation is in the language the team speaks. Looking at this list, Emre sees at a glance what blocks the merge, what is optional and what is only a question.
What a Nit Means and How Many Is Too Many
Nit is short for nitpick: a small correction request about an unimportant detail. Google's guide recommends starting such comments with "Nit:" and lists two similar labels: "Optional" (or "Consider") for ideas that may be good but are not required, and "FYI" for information not expected in this pull request but useful to know in the future. The shared goal is to keep the author from treating every comment as mandatory.
Nits have a dose. One or two in a review is reasonable. Ten nits say one of two things: either a tool is missing (formatting, naming format or import order are a formatter's or an analyzer's job), or the reviewer is applying personal taste as if it were a team rule. In the first case the right move is not to write comments but to set up the tool; that is what the Coding Conventions and Team Standards series was for. In the second case, gathering the nits into a single summary comment that says "all optional" lightens the author's load.
Timing matters too. Ten nits on a newcomer's first pull request scare more than they teach. Google's paper shows that changes from people who recently joined the company get more than twice as many comments as those from experienced colleagues; that is natural, but the comments should go mainly to blocking problems and real learning opportunities.
Replying to Comments as the Author
Half of a review is the author's reply. Emre leaving a reply on every comment saves Ayşe from guessing what was done in the second round. Short replies are enough: "Fixed, 548de4c" or "Added a backfill to the migration and the deploy note to the description". The commit hash lets the reviewer open the related change directly.
If you disagree with a comment, say so with your reason and leave the decision open. Google's guide asks the same openness of the reviewer: the author often knows the code better than the reviewer; if they have a valid point, the reviewer accepts it and drops the issue. What matters is that the disagreement stays on the table as two technical arguments, not as a personal matter.
Beware of "I'll fix it later". Google's experience is that most cleanups put off for later never happen; the guide recommends doing the cleanup before the code is merged, or immediately filing an issue the author assigns to themselves. Bookstore's rule is simple: if you say "later", write the issue number too.
Who closes the conversation? On GitHub each comment can be "resolved" as a conversation. In Bookstore the author who made the fix resolves the conversation when sure the fix is what the comment asked for; comments that involve a discussion are resolved by the person who wrote them. The ruleset setting we will see in the fifth post can block the merge while a conversation is open, which makes who closes what matter more.
How to Handle Disagreement
Emre objects to adding CategoryId to BookInfo: he does not want the Ordering module to know about categories; if Catalog moves to its own service one day, this field will be one more dependency. Ayşe thinks Ordering going straight to Catalog's table is the bigger dependency. Both arguments are reasonable. The path to follow is, in order:
First, data and documents. A technical fact or a measurement outweighs opinion. The query count has been measured; the module boundary rule is written in CatalogService's comment. On style, the team's style guide has the final word; where the guide says nothing, the existing code's consistency or the author's preference applies. If two designs are equally valid, Google's guide recommends accepting the author's preference.
Then, a synchronous conversation. A written argument longer than three messages usually ends with a fifteen-minute call. The rule here is to write the outcome into the pull request: "We talked; we are adding CategoryId to BookInfo, a field that will be in the contract anyway once Catalog is separated." A decision made in a meeting and not recorded gets argued again six months later.
Then, the decision owner. If two people cannot agree, who decides is known in advance: in Bookstore the tech lead, otherwise the module's owner. This is not a defeat; it is a rule so the pull request does not get stuck on a disagreement.
And if needed, an ADR, a short document that records an architectural decision with its context and consequences. If the disagreement goes beyond this pull request and turns into a general question such as "how do modules read each other's data?", it is not settled in the pull request; a short decision record is written and the pull request is fixed according to it. How to write one with the Nygard template and MADR is in the Architecture Decisions series, in "Writing ADRs: Decision Records with the Nygard Template, MADR 4.0 and Y-Statements", and how decisions stay alive in a team in "Keeping Decisions Alive: Decision Logs, Architecture Reviews and the Advice Process" in the same series.
Review Fatigue and Bottlenecks
Bookstore has two developers; every pull request from Emre goes to Ayşe, and hers go to Emre. When the team grows, the most common picture is different: more than half of the reviews pile up on one or two people, usually the seniors who know the code best. They spend half their day reviewing, their own work slips, and their reviews get shallow. Everyone else waits.
The first sign of fatigue is waiting time. If the one-working-day goal stops holding and the waiting pull requests always sit with the same person, that person is the bottleneck. The second sign is the LGTM reflex: approvals go up, comments go down, large pull requests are approved in minutes. That is exactly the complaint in Bacchelli and Bird's interviews: looking at what is easy to find so the review counts as done.
The measures that work are simple. Setting aside two fixed time slots a day for review keeps the one-working-day goal without breaking focused work. Choosing the reviewer by rotation spreads the load and spreads knowledge of the code; on GitHub, automatic review assignment for a team can do that, and we will look at its settings in the fifth post. Limiting how many pull requests one person keeps open at a time shrinks the pile waiting for review. And the most effective, the subject of the earlier post: small pull requests. An 80-line pull request is read in ten minutes; a 600-line one waits until the evening.
A junior reviewing a senior's code is one way to spread that load, but it has another benefit. A junior's comment "I didn't understand this part" is a valuable finding: if the person who knows the code base most recently does not understand it, anyone opening the code six months later will struggle too. Google's guide also says explanations written in the review tool do not help future readers, and that code that is not understood should usually be made clearer. What the senior should do here is not leave the explanation in a comment but change the code or the name.
In Production
In teams working in different time zones or at different hours, every review round means a day's delay. Two habits help: sending all comments at once (on GitHub, collecting comments with "Start a review" and sending them in one notification) and approving with small remaining comments. The author makes the fixes and merges without waiting for a second round.
The team's language is a decision too. In a Turkish-speaking team, writing review comments in Turkish is natural and fast; contributing to open source or to a repository shared with a foreign team needs English. What matters is that the comment language does not get mixed up with the language chosen for code and commit messages, and that the labels stay fixed in every case.
Reviews now also contain comments written by AI tools. We discussed their tone and place in the AI-Assisted Development series, in "Reviewing and Verifying AI-Written Code: Typical Failure Modes, a Checklist and AI Reviewers"; the only note here: accepting or rejecting a bot's comment also needs a reply, and bot comments left without one make human comments invisible.
Common Pitfalls
Silent approval. Writing "LGTM" and approving without opening the diff leaves the review agreement on paper. An approval should say at least in one sentence what was checked.
Imposing personal style. "I prefer a loop to LINQ" is a preference, not a rule. If it is not in the team's guide and does not harm code health, the comment is at most a labelled nitpick.
Sarcasm and exaggeration. "Great, another N+1" does not carry irony in writing, but it does carry hurt. A serious problem is written in a serious, plain sentence.
Treating every comment as blocking. Twenty unlabelled comments look like twenty mandatory tasks to the author. Stating clearly what blocks protects both the author's and the reviewer's time.
Dragging out a written argument. If the same topic has reached its third message, the conversation should continue synchronously and its outcome should be written into the pull request.
A Small Experiment
★☆☆ Reread the last five review comments you wrote. How many address the person and how many the code? How many give a reason? Rewrite the ones without a reason so that they have one.
★★☆ In your next review, write every comment with Conventional Comments labels. Could the author tell which comments block the merge without asking? Did you write at least one praise by the end of the review?
★★★ Work out the reviewer distribution in your team's pull requests over the last month: how many reviews did each person do, how many hours passed on average until the first review? If the load piles up on one or two people, write the team a proposal for rotation and review hours; open your proposal as a pull request and put it up for review too.
The language of comments and handling disagreement are people's work. But part of the review process, who reviews which file, how many approvals are needed, blocking the merge while comments are open, is set up through the platform's settings. In the last post of the series we move to GitHub: CODEOWNERS, rulesets, suggested changes, merge queue, bots and measuring the review process.