Commenting and Approving Pull Requests

After reviewing a lot of pull requests, I’ve settled on a simple default: if my comments are all nitpicks, suggestions, questions, or non-blocking issues, I leave them and approve the PR at the same time.

Here’s some detail on how it works. But first, two clarifying questions.

Why Leave Comments, If Approving?

Why leave comments if I’m approving?

Comments show that someone has thought about the problem and the solution, and cares about what’s happening to the code. Occasionally, they offer a chance to learn or to surface misunderstandings, assumptions, or hidden risks.

I almost always leave a comment on each PR I review, even just observations: “This class is getting big, we might want to consider adding a presenter,” or praise: “Thanks for cleaning this up!”

Why Approve, If Commenting?

Why approve, if I’ve left comments that I think are worth implementing?

Because I trust my team. I know that my comments will be considered, and if they’re useful, implemented.

My team is fast enough to make the change. CI runs fast enough to validate it. The time it takes to do it isn’t an obstacle.

Process Points

Here are a couple of process points that this workflow assumes.

First, you have to trust your team. If you don’t trust them to read and think about your comments— if they aren’t a strong enough signal— work on that.

Also, some repositories are configured to reset approvals when a new commit is pushed to the PR branch. If that’s your configuration, this approach is less effective because your review is removed when a change implementing your suggestion, or anyone else’s, is committed. There’s still value there because your approval is documented on the PR, but it’s not ideal.

Additionally, some repos can be configured to automatically merge PRs when all requirements are met, one of which might be your approval. Consult your repo configuration before taking action.

This process also works better with low-configuration tooling that negates a lot of trivial nitpick comments. With linters, auto-formatters, type checkers, security scanners, etc. running in development and CI, some lower-value comments don’t have to be written.

Finally, all the comments I write are contextualized by Conventional Comments. I use labels like nitpick:, question:, suggestion:, and issue (non-blocking): to clarify intent as I approve.

Here are a few other tools, like Conventional Comments, that have been recommended to me:

• The MoSCoW Method
• Netflix’s Feedback Ladders
• Emojis (👁️, 🧹, 🐛)

What About Blocking Comments?

For more major feedback, the kind that should block work, I might comment only, or comment and block.

It’s something I decide case-by-case, and it’s different for different kinds of issues, projects, and the person whose code I’m reviewing.

Code reviews are an expensive place to find a blocking issue or design flaw. On the kind of teams I’ve worked on the most, startups and SMBs, this frequently points to an upstream misalignment. Other teams might have stricter code-review gates, and it’s more common.

You want to get to a place where most feedback is non-blocking because the team is highly aligned. And then you can comment and approve with confidence.

Challenge: Comment and Approve!

For the skeptical, my suggestion is to try this with someone on your team with whom you have rapport. You can say in a comment on the PR: “This looks good, approving. I think that the rename is worth doing, but otherwise good to go.” Most engineers I know will consider this advice and do it. Conversation over, and code probably better.

I want my code reviews to coach and teach, and this practice helps. When it errs, it errs on the side of progress rather than process.

It’s my way of saying: “I care about this change, and here’s how I think it could be even better. It’s your call when to ship it.”