Excellent Pull Request Reviews

05 Feb 2025 in Power Skills

Pull request reviews are a critical part of building high-quality products, but too often, they become a rubber-stamping exercise—skim, “LGTM,” approve. This kind of review can lead to broken code, unclear documentation, and missed opportunities for improvement.

A great PR review isn’t just about glancing at the code; it’s about running the changes, thinking critically about what’s missing, and providing actionable feedback.

Run the thing

If I could only have one rule for reviewing pull requests, it would be “did you run it?”. Far too often I see people skimming over the content, giving it a “LGTM” and pressing approve.

This happens everywhere. My most recent example is a project that is migrating documentation from one platform to another. All of the content already exists, so it’s just about reformatting and publishing it elsewhere, right?

No. No, no, no. You see, a lot of the documentation doesn’t work. Maybe it didn’t work when it was originally published (“LGTM!”). Maybe it did, and it’s rotted over time and no longer works.

So now we have a new definition of done for the migration work. Before you submit a pull request for review, you need to work through the content from top to bottom and ensure it works. Then your reviewer has to do the same.

Having two people test the same thing ensures that the documentation is clear, isn’t missing implicit knowledge and most of all, that it works.

If you’re building software rather than writing docs, good news, the same process works! Having two people use the software makes sure that it meets the requirements, uncovers edge cases and generally helps build better products.

What’s missing?

Looking at a pull request and saying “LGTM” is only half of the review. You also need to think about what you’re not looking at. What should be there, but isn’t?

Are there any missing tests? Missing docs? How about example configuration or usage examples for the feature that you just added?

Spotting what isn’t there is one of the hardest things to learn when reviewing PRs, but it’s also one of the most impactful. Being able to say “this looks great, but I think we should add a test for when X” helps build better products.

Is this blocking?

As you add more comments to a pull request (and you will add more comments if you’re actually running what’s there and thinking about what’s missing), it can be hard to understand which pieces of feedback are opinions, and which require changes.

To make it obvious for the original author, I recommend prefixing feedback if it doesn’t need to be actioned immediately.

  • No prefix: This is important feedback for the current PR
  • future: Could we refactor this into something that’s reusable?
  • nit: I find it easier to parse when using early returns rather than else blocks

By following this pattern of feedback you can ensure that you provide all the context you have in the PR without worrying about preventing the original author from shipping.

I’ve been on the receiving end of this type of feedback, and it’s been great to learn what could be done given more time. I don’t always go back and fix the nit comments, but it always makes me think about what I can do differently next time.

Do the work

Last, but by no means least, do the work yourself. I don’t mean “pull down the branch and make changes” (though in at least one of my teams that is the default operating model and it works well).

I mean use GitHub’s suggestions macro when reviewing a pull request. If you have a suggestion how to improve something, instead of trying to explain it in text you can edit the lines yourself using the following format:

bash
```suggestion
This line will be suggested instead of the one that you selected
```

Not only is making the actual change easier to parse for the original author, they can quickly apply any changes they agree with by clicking “add to batch” in the web UI.

Go forth and review

By making sure the code works, thinking critically about what’s missing, and clearly distinguishing between blocking and non-blocking comments, reviewers can significantly improve the quality of the product. And when possible, making direct suggestions instead of just pointing out issues can streamline the process for everyone.

A great review isn’t just about catching issues. It's about fostering collaboration, improving maintainability, and ultimately, building better products. So next time you review a PR, take the time to do it right. Your team (and your future self) will thank you.