I log into Github and notice I’ve got a nice blue badge indicating that I have an unread notification. A member of my team has created a pull request for one of our projects and has requested a code review.
I go through the pull request and nothing seems amiss. There aren’t any glaring security issues, all the code is formatting correctly, and the code makes use of React and Redux design patterns which we were using. I comment on the pull request with some minor thoughts on things we may need to consider in the future, approve the pull request, and merge the branch into master.
I have even more confidence in the merge because the CI server has said that it passes all the lint rules, passes all the unit tests, and all of the storybook stories still work. I review and merge the pull request in under 5 minutes because I know I don’t have to check for trivial whitespace issues, or function names, or a multitude of other minor issues. I stay high level and make sure the code is clear, concise, and edge-cases are handled.
But this didn’t just happen over night. I made sure that as we started growing the front-end team, we would always do code reviews — even if we were busy and it was a quick once-over. This helped new team members feel confident in the code they were writing. We all understood new code that was going into the system. It also let people share insights they had when writing new features or fixing bugs.
So how do you get to the point where code reviews happen smoothly?
The hardest part is just getting started. At first, the code reviews will be very informal. If you aren’t telecommuting, I’d recommend having them in person and having people walk through their code, what they did, and why they did it. It’s not an inquisition though. Make sure the mood is laid-back. The person whose code is being reviewed shouldn’t feel like they are defending themselves, but rather, sharing decisions that were made when writing the code.
The problem with getting started though is that if you haven’t had code reviews before, there may be a lot of bickering over coding style, or design patterns used, or a multitude of other things.
If you find yourself bickering, stop and write it down, because you are going to want to:
When you first start doing code reviews, you may find yourself going back and forth. The code reviewer asks for some lines of code to be cleaned up. The coder goes and cleans up the lines. The reviewer then comments that they didn’t fix it just right.
These cycles of review and fixing are just wasting time. They waste the time of the reviewer who could be writing new features, and they waste the time of the reviewee who just wants to move on. These type of comments tend to seem arbitrary and they don’t share any meaningful insights to anyone.
Use a CI tool to automatically check and fail for trivial things: coding style, failing tests, bad function names, “banned” functions (like eval). Don’t waste time when you can get a CI tool to automatically look for these things.
Once you get your CI tool set up, start adding new checks to it when you find yourself and your team bickering about something trivial like BEM versus OOCSS style names.
Make it part of the process and culture that code reviews are necessary for creating new features. Make everyone do them. Have junior developers review a senior member’s code. Sometimes they have insights about language features that may save time or are more performant.
Remember, code reviews aren’t about nitpicking through another person’s code. Use a computer to nitpick. Trust me, they are really good at it. Your coworkers’ time is valuable, so use it wisely. Focus on security issues, edge-cases, optimizations, and architecture.
The culture of your code reviews is just as important is doing the code reviews. Make sure you are focusing on what matters and not bikeshedding and your team is instead using code reviews as a chance to learn, improve, and create amazing features.
Thanks for reading! If you liked this article, feel free to follow me on Twitter.