Sean KilleenI often do a lot of code reviews and pull request reviews. As a follow-up to my article on submitting pull requests, I wanted to talk about how I review them as well.
First – A Few Caveats
- This is my style; yours might vary, and that’s fine! I think there are a lot of ways to do this well, and there’s no sense in being dogmatic about it.
- I don’t follow this 100% every time. I don’t think you have to do these things exactly in order to be successful. It’s better to adapt to your situation/context.
- I’m assuming you’re familiar with pull requests and how they work. If you’re not yet familiar with that, I recommend searching around and finding some great articles on that first, or some of the contents of this article might be confusing.
- This advice is geared less toward public OSS projects. In those cases, there are additional requirements for review IMO, because the OSS authors will have to maintain that code in perpetuity after the change is merged, and the original author may or may not be available. This article assumes we’re talking about a team project.
- This assumes you want pull requests. Many teams prefer to forego them because they feel they add unnecessary overhead. I disagree; I think they build historical context and communication skills that pay off in the long run. My point is, if you doubt pull requests in general, then this post isn’t for you.
Before Beginning – the Right Mindset
I try to always keep a few things in mind prior to reviewing code:
- Reviewers should be rooting for those they’re reviewing. I should want the person whose code I’m reviewing to succeed.
- Code always has a context, and no code/author is perfect. This perspective is important, I think. Too often, we review pull requests outside of the context in which they’re offered.
I find reminding myself of these things put me in the right mindset to add constructive, helpful comments.
