mb bischoff

is a work in progress…

Reviewing Code: A Checklist

You’ve been there. A 10,000 line pull request lands in your email and you don’t even know where to begin. No description provided.

Should you start by installing it, running the test suite, or should you just start scrolling though while your eyes glaze over from the red and green stripes? Is this developer really looking for feedback or are they on a deadline and pressuring you to say “lgtm”. Will your one innocuous comment ignite a flame war?

Code review, or more generally peer review, has a long record of finding defects in not just software engineering, but in science, academia, and many other industries. While some argue about the specific percentages of issues that it finds compared to an automated test suite, or others method of testing, code review is an incredibly useful tool in any software teams arsenal against bugs and poor software architecture. But only if it remains a sharp one. Laziness can creep into code review very easily if you’re not careful.

Jason Brennan has written two great pieces on the topic of diligently writing and reviewing pull requests, and I’ve used his series as an inspiration for my own system. I’ve also found it’s handy to have a short-form checklist to go over when performing code reviews to remind myself what I’m going for. Maybe it’ll help you and your team too.

For the engineer drafting the pull request

For the reviewer

I find it’s helpful to do these in order if you can.

Most importantly, remember that the engineer on the other side of the screen is a person. Try not to be curt or hurtful in your comments.

The Pull Request

Start here to build an understanding of what you’re about to be diving into.

The Product

Does it work as you’d expect? You can write lots of pretty code that doesn’t work.

The Code

Alright, it’s time to start looking at the code, line by line.


Adding a system like this to my own pull request reviews has helped me catch a lot more, but I’m sure there are still things I’m missing. If you have ideas about how this checklist can be improved, pull requests are welcome!

Filed to: Longer, TechTagged: , , , ,