Code review comments that teach, not sting

A code review comment is a message to a person, and it sits in public, attached to something they made. Write it that way: say what you would change and why, in the same comment, before anyone has to ask you for the missing half.

Most review comments read like compiler output instead. A location, a terse verdict, no reasoning, no author. The compiler gets away with that tone because nobody has ever felt judged by a compiler.

Put the reason in the comment, not in your head

Every comment should carry the change you want and the reason you want it, together, because the reason is the only part that survives past this diff.

"Use a map here" is a verdict. "Use a map here, the lookup runs inside the request loop and the list grows with the account" is a review. Same edit, different message entirely.

Without the reason, the author makes the change, learns nothing they can carry, and writes the same line next week in a file you are not reviewing. With it, they can apply the idea in places you will never see. That is the whole return on the time you spend reading someone else's code.

The reason also gives them something to argue with. A bare instruction can only be obeyed or resented. A stated reason can be wrong, and occasionally mine is, and finding that out beats getting my way.

What a finished comment holds:

• The specific change you would make.
• Why it matters in this code, not in general.
• How confident you are.
• What you would accept instead if they see it differently.

Writing the reason has a side effect I did not expect when I started forcing myself to do it. Some of the time I start typing the why, notice the why is thin, and delete the comment before it ever posts. That is the habit paying rent.

Label severity honestly, then leave fewer of them

Say whether a comment blocks the merge, because an unlabelled comment defaults to blocking inside the author's head.

Three levels cover nearly everything: this blocks, this is worth fixing before you forget, this is my preference and I will not raise it again. One word at the front of the comment does the job.

Here is where I will lose some of you. Nit-labelling is overrated. Twenty comments tagged "nit" still reads as a heavy review, because the author feels the length of the scroll bar long before they read a single label. Volume is its own message, and no tag outranks it.

So the labels only work if you also cut. When I have more than two or three small style points, I either fold them into one comment or drop them and raise the pattern in conversation later. The general craft of written notes applies here too, but review has its own trap, which is that leaving a comment costs the reviewer four seconds and the author four minutes.

The related opinion, and I hold it firmly: approving with comments is usually the honest default. If you would be happy with the code once they make the fixes, approve and say so. Holding the approval hostage so they come back and show you their homework is a power move wearing the costume of rigor. Block when you genuinely need to see the next version. That is rarer than review culture pretends.

"Why not use X?" is almost never a question

Ask a question only when the answer could change what you want. Otherwise write the statement, because the disguised version makes the author explain themselves before they can start working.

"Why didn't you handle the empty list?" reads as curiosity and functions as an order. It means handle the empty list, plus a quiet demand that they account for the omission first. Now they either defend a choice they never consciously made, or comply while feeling told off. Neither is what you wanted.

Translations that cost nothing:

• "Why not use the existing helper?" becomes "There is a helper for this in the utils file, use that unless it does not fit."
• "Is this tested?" becomes "I cannot find a test for the failure path, worth adding one?"
• "Do we really need this abstraction?" becomes "One caller today, so I would inline it, happy to be told there are more coming."

Real questions belong in reviews, and they are often the best comments in the thread. "What happens if this retries while the first call is still open?" is a real question when you do not know the answer. You will know it after they reply, and so will the next person who opens the file.

The test takes a second. Could their answer change my request? If yes, ask it. If no, you had an instruction, and the costume only adds a round trip.

Your mood ends up in the diff, and the diff keeps it

Reviews written tired or annoyed read exactly like that, and unlike a remark at someone's desk, they stay bolted to the work for as long as the repository exists.

I can tell which of my reviews got written at the end of a long day. The comments get shorter and the questions get pointier, and the word "just" turns up everywhere: "just use the built-in", "this could just be one pass". "Just" is the tell. It means the fix looks obvious to me and I am wondering out loud how you missed it.

The other reliable trigger is the third pass over the same pull request. By then you are not really reviewing the code. You are reviewing your own patience, and it shows up in the punctuation.

My fix is boring. Write all the comments, then walk away and come back before submitting, and reread only my own first sentences. That pass usually deletes two comments and rewrites one, and it costs about a minute. When the minute is not available, I would rather send the review a notch warmer than I feel, because that direction has never once come back to bite me.

This is the small thing Unvent does for me inside a comment box: the technical content stays exactly mine, and whatever the last hour did to my tone comes off before anyone else reads it.

Know when to close the tab and go talk

When a thread reaches its third round, the thread has stopped being the right place, and the fix is a call rather than a better paragraph.

The signs are easy to spot once you look for them:

• You are restating the same point in new words.
• The disagreement is really about approach, and this code is just where you noticed it.
• Someone starts quoting earlier comments back at the other person.
• The sentences keep getting more polite as they get colder.

That last one is the strongest signal I know. Elaborate courtesy in a review thread means the actual conversation has moved somewhere the thread cannot follow, and no amount of careful wording brings it back. Some disagreements can be settled in writing. Approach arguments in a comment box mostly cannot.

After you talk, post the outcome in the thread, in one plain comment. "Talked it through: keeping the current approach for now, opening a ticket for the retry case." Threads outlive everyone's memory of them, and a resolution that happened in a call and never made it back is a small theft from whoever reads it later.

The reviewer who taught me the most left very few comments, and each one gave me something I could use in the next file I opened, well beyond the one under review. That is the bar I aim at and often miss. Would this comment still be worth reading if the code it points at were deleted tomorrow? If yes, it taught something. If no, it was maintenance, and maintenance can be short.