How to write code review comments, with before-and-after examples
Learn how to write clear, respectful code review comments and show whether a change is required or optional. With four before-and-after examples and how to speak your comment with Yappee.
You're reading a pull request, you trip over one spot, and you quickly type "Why did you do it this way?". You meant it as a real question. What arrives is an accusation. A review comment is the short piece of feedback you write on a line or a section of a proposed change. Tone of voice, facial expressions, and body language are all missing, along with the quick follow-up question in passing. Here the tone comes only from your word choice. It also decides whether it's clear what needs to happen. You'll learn how a helpful comment is built, how to separate what's required from what's only a suggestion, and how to rewrite four typical comments.
What goes into a helpful comment
Before we get to labels like "Nit:", a simple frame of four parts helps. Not every part belongs in every comment. All examples in this article are made up, including the file, function, and variable names in them.
- The code, not the person. In its engineering practices, Google recommends writing comments that are kind, clear, and respectful. On touchy points in particular, the comment should be about the code and not about the person who wrote it. "You misunderstood the cache" then turns into "
loadProfilereads the cache even for expired entries". - The specific observation. Name the file, the function, and the case you mean. "Something's missing here" forces the other person to guess, and guesses usually go wrong.
- The reason, where it isn't obvious. A reason can explain the intent behind your point, a practice your team follows, or the benefit for the code. Google says explicitly that not every comment needs an explanation like this. A typo is still a typo.
- The next step. Here you weigh naming the problem against prescribing the solution. Google says the person who wrote the change stays responsible for fixing it. They're closer to the code and can therefore find the better solution.
What "Nit:", "Optional:", and "FYI:" mean
Plenty of comments leave one question open. Do I have to change this before the change goes through? Two common systems answer it with a short prefix.
Google's guide to review comments names three labels. "Nit:" marks a small point that should still be changed technically. "Optional:" or "Consider:" marks an idea that isn't necessary. With "FYI:", you expect nothing in the current change. The note is just something to think about.
Conventional Comments proposes a firmer form, <label> [decorations]: <subject>, with an explanation below it when needed, covering context, reasons, and next steps. Four labels cover most everyday cases. "issue" names a problem, "suggestion" a proposed improvement, "question" a possible difficulty whose importance you can't judge yet, and "nitpick" a small request that comes down to taste. On top of those come additions in parentheses. "blocking" holds up approval, "non-blocking" doesn't.
One catch remains. "Nit" doesn't mean the same thing everywhere. At Google, the change is small but wanted. In Conventional Comments, "nitpick" is a matter of taste and doesn't block. If you use the word without agreeing on it first, you're relying on a meaning the other side may have learned differently.
So your team's convention comes first. Conventional Comments explicitly allows you to diverge from its list, and for the additions in parentheses it advises a small, unambiguous set. If you use no labels at all, a clear sentence at the end of the comment does the job, such as "This should go in before the merge" or "Just a suggestion, nothing required".
Four comments before and after
Each example shows a comment as it often gets written, and then a version with the four parts.
Missing test case
Before: "Tests are missing here."
issue (blocking):
parseDurationhas no test for negative values. That's exactly where the bug with the negative times came from in the last release. Could you add a case for it?
The comment names the function, the untested case, and the reason. And it says that approval depends on it.
Suspected bug
Before: "This can't work like that."
question:
sendReminderpassesuserIdon without checking it. What happens when the call comes from the nightly job? I don't know that part of the code well enough.
A verdict turns into a question that really is one. The last sentence says openly why you're asking.
Unclear name
Before: "Bad name."
Nit:
data2says little about what's in it. How aboutnormalizedRows? No reason to hold up approval.
That last sentence clears up the ambiguity of "Nit" without your team having to agree on a system first.
Optional rewrite
Before: "You could solve this in a completely different way."
Optional: The four
ifbranches inresolveTiercould be written as a table of the thresholds. New tiers would then be one row instead of one branch. Doesn't have to go into this pull request, though.
All four versions name a place, a behavior, and how important the point is. In most cases nothing more is needed.
Ask real questions and bundle small points
In its notes on written feedback, Conventional Comments advises asking where you genuinely lack context. That's different from an instruction dressed up as a question. "Don't you think this belongs in its own function?" sounds open, but it leaves the other person no choice and makes them guess at your intent. If you want the change, write it down. "Please pull this into its own function, otherwise the method does three things."
The second point is about small stuff. Ten separate comments about the same formatting question produce ten notifications and make a review look bigger than it is. Bundle similar points into one comment, give one or two examples, and say how they can all be resolved in one go.
Check every comment at the end against one question. Can the other person tell from it what they need to do to close it out?
Name what worked and say why
Google also recommends commenting on the parts you like, and saying why. The reason makes the difference. "Looks good" is decoration. A specific reference is information.
praise:
rejectsExpiredTokenexplains the expected behavior better than any comment could. I'm going to copy that structure for the other auth tests.
None of this is mandatory, and it doesn't replace a technical point either. But an honest sentence about the right spot costs little.
Speak your review comment with Yappee
Putting it into words can take more time than reading the code. Ten seconds in, you know what bothers you, and then you type three attempts until the sentence sounds neither harsh nor vague.
For that, Yappee has the "Code Review Comment" format. You speak your feedback and get a review comment that you paste into GitHub, GitLab, or a similar tool. Yappee runs on iPhone and Mac and supports more than 50 languages for transcription and translation. If the review runs in English, you can speak in your own language and have the comment come out in English.
- Say the place, the observation, the reason, and how important the point is. Those are the four parts from above.
- Choose the "Code Review Comment" format and read the text through.
- Before you paste, check the file, function, and variable names, and the label your team has agreed on.
Yappee works with what you say. If your observation stays vague, so does the comment. The app doesn't hook into GitHub or GitLab, so you paste the text yourself. And for a "Nit: typo", typing is faster than starting a recording.
The pattern behind it stays the same whether you speak or type. Name the place, say what you noticed, add the reason when it isn't obvious, and make it visible whether you expect a change.