Sascha Mirow-Stanitzok

Posted on Oct 28 Originally published at sascha-ms.de

Code Review Feedback: Comments That Land

#career #leadership #softwareengineering

Code review feedback is where engineering and leadership touch most directly, at least in my view. At first glance, a pull request is a purely technical matter: read the diff, spot the bugs, approve. In reality, one person reads another person’s work and writes something next to it, usually in text, often under time pressure and without any tone of voice. No wonder so much frustration starts right here.

My article “Giving feedback that lands” was about feedback in conversations. This post takes those ideas to the place where developers give and receive feedback every day: the comment column next to the code. You get phrasings you can borrow, a simple system that makes the weight of a comment visible, and a template for a review agreement in your team.

In short

Good code review feedback talks about the code rather than the person, explains the why and states clearly whether a point blocks the merge. Labels such as “issue (blocking)” or “nitpick (non-blocking)” remove a lot of guesswork from written comments. The biggest stumbling block is a lack of clarity: when nobody knows what is mandatory and what is taste, every comment turns into a power struggle.

What you will learn in this article

Why written code review feedback is so easily misread
How to build review comments from observation, impact and suggestion
How Conventional Comments show what blocks a merge and what is just a hint
A template for your team’s review agreement
How to handle criticism of your own code calmly as an author

Why code review feedback goes wrong so easily

A comment like “Why so complicated?” takes three seconds to type. Maybe it was meant as honest curiosity. It gets read in the evening, after a long day, by someone who spent two days on exactly this solution. In a conversation, voice and facial expression would have softened the question. In a pull request, only the bare sentence remains, and our minds happily fill the gap with the least favorable tone.

On top of that, there is a power gap that rarely gets named. Whoever reviews has a say in whether someone else’s work may enter the main branch. Every comment therefore carries an unspoken question: do I have to change this, or is it just an opinion? If that stays open, many authors change everything just to be safe, and the review feels like an exam instead of collaboration.

What code reviews are actually for

A look at research helps to sort out expectations. In 2013, Alberto Bacchelli and Christian Bird observed and surveyed developers at Microsoft and analyzed hundreds of review comments. Finding defects was the top motivation, yet reviews turned out to be about defects less often than expected. Much of the value came from elsewhere: knowledge got shared, the team stayed aware of what was changing in the code, and alternative solutions emerged. At the center was always the effort to understand the change in the first place.

For your code review feedback, this means that a review is also a place to learn. If you only hunt for bugs, you give away the larger part of the benefit. If, on the other hand, you ask questions, explain reasons and name good solutions, you spread knowledge across the team along the way.

Talk about the code, not the person

The code review guidelines that Google has made public start their section on comments with a simple rule: be kind and talk about the code, not the person who wrote it. That sounds obvious. In everyday work, though, a “you” easily slips into a sentence where it isn’t needed. “You forgot the error handling here” points at a person. “The error handling for an empty response is still missing here” points at a line of code.

Explaining the why matters just as much. A comment without a reason is an order, and orders create either resistance or blind obedience. You want neither. With a reason, the author can check whether your argument even applies here and push back if they have more context than you. The following table shows typical phrasings and an alternative that addresses the same problem.

Instead of	Try
“Why so complicated?”	“It takes me a while to follow this loop. Would a `filter` with a descriptive name be easier to read?”
“This is wrong.”	“With an empty list, `list[0].id` throws an error. An early return would catch that.”
“You forgot the tests again.”	“The new branch still needs a test. Would you add one?”
“Just use library X.”	“With X you could drop the custom retry logic, because it already comes with backoff. What speaks against it from your point of view?”
“Ugly.”	“This function currently does two things: validate and save. Split up, both parts would be easier to test.”

Typical review comments and a phrasing that stays with the code and delivers the why

Also watch out for small words with a big effect. “Just”, “obviously” and “again” imply that the other person should have known better. Without them, the same sentence becomes more factual, and nothing is lost.

Review comments with observation, impact and suggestion

In feedback conversations, I like to use the SBI model: situation, behavior and impact. It translates well to a pull request. The situation is already settled, since the comment is attached to a specific line. That leaves three building blocks. What do you observe in the code? What impact does it have, for example on users, operations or maintainability? And what do you suggest, or what is your question?

An example from a made-up library app makes this concrete. Someone has added a function that lets members extend a loan:

// src/loan.ts
+export async function extendLoan(loanId: string, days: number) {
+  const loan = await loans.load(loanId);
+  loan.dueDate = new Date(loan.dueDate.getTime() + days * 86400000);
+  await loans.save(loan);
+  return loan;
+}

The code works, but it has a business gap: if another member has reserved the book, the loan is extended anyway. A quick comment would be “Forgot reservations?”. With observation, impact and suggestion, it looks like this:

issue (blocking): Loan can be extended despite a reservation

The function also extends the loan when another member has reserved
the book. That person then waits longer without being told.
Suggestion: before saving, check whether a reservation exists and
throw a domain error in that case. Would you add a test for it, too?

The comment is longer, but it saves a round of follow-up questions. Anyone reading it knows right away what the problem is, why it matters and what a fix could look like. At the same time, it leaves room for a better idea, because the suggestion is phrased as a suggestion.

Suggest small changes directly

For small things, a long explanation is too much. On GitHub, you can add a suggested change to a line comment, which the author can accept with a single click. To do this, write the new code in a block with the keyword suggestion:

nitpick (non-blocking): The number is easier to read as a constant.

suggestion
loan.dueDate = new Date(loan.dueDate.getTime() + days * MS_PER_DAY);

That is friendly, because it saves work, and unambiguous, because no interpretation is needed. By the way, honest praise in the right place pays off as well, for example: “praise: Nice that the function returns the updated loan, that makes the test in the handler much simpler.” Google explicitly recommends pointing out good work in reviews and saying what makes it good.

Conventional Comments: what blocks and what is just a hint?

The most important question behind every comment is this: does it have to be resolved before the merge? Conventional Comments is an open format that makes exactly this question visible. Each comment starts with a label, optionally followed by a decoration in parentheses, then the subject and below it the reasoning. The format is deliberately lightweight. You don’t need any tool for it, only an agreement within the team.

The labels at a glance

Label	Meaning	Example
`praise`	Sincere praise for something that works well	“This test covers exactly the edge case we would have missed last time.”
`issue`	A specific problem, ideally with an idea for a fix	“The function crashes on an empty list.”
`suggestion`	A proposed improvement with a reason	“A map instead of the loop would speed up the lookup.”
`question`	Not sure whether there is a problem, so ask first	“Can `days` become negative here?”
`todo`	Small but necessary change	“Please add the changelog entry.”
`nitpick`	Matter of taste, never blocking	“I would call the variable `deadline`.”
`thought`	Idea for later, not for this pull request	“In the long run, reservations could become a module of their own.”
`note`	Pure information	“The constant is also used by the overdue reminders.”

The most important labels from Conventional Comments with examples from the made-up library app

There is also chore for mandatory tasks from a fixed process, such as a missing ticket. Teams may add labels of their own, too. My advice: start small. With issue, suggestion, question, nitpick and praise, you cover most cases.

Blocking or non-blocking?

The decorations in parentheses make the weight explicit. (blocking) means: no merge until this is resolved. (non-blocking) means: please consider it, but it is no obstacle. With (if-minor), you ask the author to address the point only if the change stays small. Google’s guidelines recommend the same principle in different words: mark comments with “Nit”, “Optional” or “FYI” so the author knows what is required and what isn’t.

Sorting code review feedback: the further left a label sits, the more likely it blocks the merge. Each team sets the exact line itself.

A default rule for comments without a decoration is crucial. I prefer “non-blocking unless stated otherwise”. That way, every block has to be stated on purpose, which noticeably reduces the number of mandatory points. Both variants work, as long as everyone knows the same one.

Agree on code review feedback as a team

Good phrasing improves the feedback in individual code reviews. To make it part of the team culture, you need a shared agreement. I recommend a short review agreement in the repository that the team writes together and revisits in a retro now and then. It doesn’t have to be long. You can use this template as a starting point:

# docs/review-agreement.md

## Response time
- First response to a pull request by the next working day at the latest.
- No time for a full review? Say briefly when you will get to it.

## Size
- A pull request contains one self-contained change including tests.
- Rule of thumb: around 100 changed lines. Split larger work beforehand.

## Comments
- We write comments in the Conventional Comments format.
- A comment without a decoration counts as non-blocking.
- Linters and formatters check formatting and import order, not us.
- When we notice something good, we say so with “praise”.

## Approval
- Approving with open comments is fine if only non-blocking
  points are left. We say so in the approval.

## Disagreement
- After two rounds of comments without agreement, we talk briefly.
- We record the outcome as a comment in the pull request.
- If we still can't agree, the responsible person from CODEOWNERS decides.

## Cleanup
- "We'll do it later" only counts with a linked ticket.

Each point has a reason. The response time follows Google’s recommendation to reply within one business day at most, because long waits frustrate people more than strict comments do. If you are in the middle of focused work, you don’t have to drop everything. A short message saying when the review will come is often enough. The linter rule takes debates about taste out of the review before they start. And the cleanup point builds on an observation Google describes openly: cleanups that get postponed rarely happen.

Small pull requests make feedback easier

Good code review feedback starts before the first comment, namely with the size of the change. Google’s guide names about 100 lines as usually reasonable and about 1,000 lines as usually too large, with the number of touched files counting as well. With 40 changed files, nobody can read carefully anymore. What remains are shallow comments or a resigned approval. Small pull requests, in contrast, get more careful feedback, and criticism hits less work. How to slice larger changes and maintain stacked branches is covered in “Splitting pull requests”.

A pull request template helps as well, because it answers the questions that would otherwise come back as comments. On GitHub, you create a file in the .github folder for this:

# .github/pull_request_template.md

## What changes?

## Why?

## How did I test it?

## What should the review focus on?

The last question is my favorite. It invites the review in a targeted way and signals that feedback is welcome.

When to talk instead of typing

Written comments have a natural limit. When a thread keeps getting longer after two rounds while the sentences get shorter, it is time for a call. Five minutes of conversation often clear up more than ten comments. Afterwards, record the outcome in the pull request so that later readers can follow the decision. This also applies to fundamental criticism of the architecture of a change, which belongs in a conversation, ideally before the code is written.

How to handle criticism of your own code

Code review feedback has two sides. Whoever opens a pull request shapes the culture, too. The most important thought here: a comment on the code is not a verdict on you. It is an offer to make the code better, and sometimes a mistake by the person reviewing it.

So you are allowed to push back. Google’s guidelines say explicitly that the author often knows the code better than the reviewer and may therefore be right. Explain your objection factually and show that you understood the argument. If you notice that someone doesn’t understand a piece of code, explain it in the code itself, for example with a better name or a comment. An explanation that only lives in the review tool won’t help the next person who reads the code.

Finally, something small with a big effect: say thank you for good hints. A short “Good point, done” shows the reviewer that their effort was worth it.

Code reviews with AI bots and agents

More and more teams let AI tools join the review, or AI agents open pull requests themselves, for example for dependencies or security vulnerabilities as with AWS Continuum. This changes the feedback culture in two ways.

First, bots can produce a lot of comments. The same logic as with humans helps here: a bot comment should make clear whether it blocks. Some tools can be configured to comment only above a certain severity. Use that, otherwise the team learns to skip bot comments across the board and will eventually miss the one that matters.

Second, responsibility stays with humans. Approving an AI-generated pull request means: I understood the change and I stand behind it. That is the same bar as for code from colleagues. Tone is not an issue here, by the way, so you can be brief with an agent. You should still be precise, because an agent can misread an unclear comment just as a person can.

Common mistakes in code review feedback

Some patterns show up again and again. The classic is silent waiting: a pull request sits for three days, then a list of twenty points arrives. Short, early responses are almost always better than a perfect review that comes late.

A second pattern is the taste blockade. Someone holds up a pull request over naming or brace style, even though the code is fine from a business point of view. Whatever a linter can check belongs in the linter. Whatever is a matter of taste gets marked as nitpick and does not block.

Third, a culture of questions that are really orders tends to creep in. “Wouldn’t it be better if …?” sounds polite, but leaves open whether you expect a change. Say clearly what you want instead, and mark the weight. Honest questions remain welcome, of course, when you really don’t know something.

Fourth, praise is missing. If reviews are nothing but lists of flaws, everyone will eventually associate them with stress. That is why Conventional Comments recommends looking for at least one thing in every review that deserves honest praise. The key word is honest, since everyone notices praise given out of duty.

My take

I consider code reviews one of the most underrated leadership tools there is, even for teams without a formal manager. Code review feedback shows every day how a team treats each other: whether questions are welcome, whether mistakes are embarrassing, and whether experienced people share their knowledge or use it as a hurdle. If you as a team lead want to work on psychological safety, the comment column is a very concrete place to start.

At the same time, I warn against the opposite extreme. Friendly feedback does not mean soft feedback. A bug that harms users belongs clearly marked as blocking, even if that is uncomfortable. Labels and good phrasing make such comments easier to accept, but they don’t make them less serious. If you only wear kid gloves, you push the problem into production.

And one more point where I disagree with many guides: you don’t need an elaborate set of review rules. One page of agreement, five labels and the willingness to talk briefly when there is conflict go a surprisingly long way.

If your team works remotely, the frame around reviews matters too. How to combine asynchronous reviews, check-ins and team agreements is covered in Leading remote teams: rituals that actually work.

FAQ

How do I write constructive code review feedback?

Describe what you observe in the code, what impact it has and what you suggest. Talk about the code rather than the person, give a reason for your point and mark whether it blocks the merge.

What are Conventional Comments?

An open format for review comments. Each comment starts with a label such as issue, suggestion or praise, optionally followed by a decoration such as (blocking) or (non-blocking).

How quickly should you respond to a pull request?

Google recommends replying within one business day at most. If the full review takes longer, a short note saying when it will come is enough for a start.

Does “Request changes” on GitHub always block the merge?

No. According to the GitHub documentation, the status is informational by default. It only blocks the merge when a branch protection rule or a ruleset requires approvals for pull requests.

Your next step

In your next three reviews, try just one thing: put a label in front of every comment and, where it isn’t obvious, a decoration such as (blocking) or (non-blocking). Watch how the replies change. If it helps, bring the review agreement template to your next retro and strike out together whatever doesn’t fit your team.

I’m curious about your view: which review comment has stuck with you in a particularly good or bad way, and why? Feel free to share it in the comments.

Sources

This post was created with AI assistance.

👉 Next step: More for team leads: my cheat sheets put the essentials on one printable page.

Originally published on sascha-ms.de. More articles on serverless, AI in everyday development and leading tech teams: sascha-ms.de/en

DEV Community