Cracking the Code Review, Part 1: Preparing Your Code Review
Stephen Rollins
Reading time: about 8 min
Topics:
“After experiencing the benefits of peer reviews for nearly fifteen years, I would never work in a team that did not perform them.” -Karl Weigers, “Humanizing Peer Reviews”
When was the last time you had a truly quality code review? Have you ever been frustrated by reviewing someone else’s code? In this two-part series, we will cover several pieces of advice that all of us--reviewers and code writers alike--can implement to make the whole code review process as efficient and painless as possible.
1. Minimize the features in one pull request
Just as a good function or class has a single, clear, straightforward purpose, a good commit or pull request solves one clear problem and does it well. The benefits are straightforward: Bite-sized, organized code reviews are easier for the reviewers to follow and result in more thorough reviews overall. The benefits also extend beyond the code review. Modular commits result in cleaner git history. Writing good commits also discourages the habit of making commits that fail builds, even if the overall pull request builds just fine. This makes debugging and bisecting on old changes much easier.- Strive for one major change per commit.
- Ensure each commit builds successfully.
- 1 issue in bug tracker = 1 pull request
- Only commit what matters--every line changed is a step toward a fix or feature.
Making commits for logically independent changes facilitates navigating through a pull request and also doubles as a good summary of what work was done.
2. Keep code reviews brief
There’s nothing worse than opening up a code review and seeing that it has touched hundreds of files and thousands of lines. If you want quality code reviews, don’t overwhelm your reviewers with hours of changes to stare at. Keep your commits and pull requests clean and brief, and your reviewers will be much more effective. The benefit here is clear: Smaller code reviews get more thorough reviews. Ask a programmer to review 10 lines of code, and they’ll find 10 issues. Ask them to do 500 lines, and they’ll say, “It looks good.” Key habits:- Merge incremental changes often.
- Avoid asking reviewers to stare at your code for hours.
The less code in your code reviews, the more thorough your reviewers can be.
3. Review your own code
No one knows your changes better than yourself. This familiarity is not always a good thing; it will cause you to miss bugs occasionally (that’s why we have code reviews!). On the other hand, your familiarity means you aren’t slowed down by having to learn a feature’s new behavior while reviewing the changes you made. Nothing is quite as embarrassing as missing a really obvious bug before handing your code over to another developer to review. Save yourself the pain by doing your own quick review first. You may be surprised what you find, and your reviewers will surely be grateful for the higher-quality code they review as a result. Key habits:- Review your code as you write it.
- Review your code as you commit it.
- Review your code before sending off a pull request.
Some developers refer to “Rubber Duck Debugging”, the practice of explaining your code to an inanimate object such as a toy duck, as helpful way to find bugs in your own code.
Code reviews are about quality, not finding fault. The point of software code review is to eliminate as many defects as possible, regardless of who "caused" the error.
4. Let it build before assigning reviewers
This tip is similar in spirit to #3. After you’ve done your own review, let your build and testing tools do what they do best: build and test your code. Don’t waste your reviewer’s time with code full of errors that are best left to computers to find, and especially don’t spam their inboxes with repeated messages of updated pull requests as you fix lingering bugs and build failures. Make sure your code is as close to the main branch as you can. It is incredibly frustrating to have a commit that builds fine locally but conflicts with upstream changes in ways that cause build errors. Key habits:- Merge the main branch into your branch as often as is reasonable--and be sure to do so just prior to a code review.
- Build your changes before giving your code to reviewers.
Don’t waste your reviewer’s time with code that doesn’t compile or pass regression tests
5. Pick the right people
A code review will not be helpful if the developers performing the review do not know the behavior and gotchas of the code they have been asked to examine. Figure out who the feature experts or stakeholders are in the code you changed, and make sure the pull request gets their stamp of approval. Are you the only feature expert? Perhaps it’s time to pull someone else on board and help them become more familiar with your part of the code. Redundancy of knowledge is always a good thing, and it makes your code reviews much more effective. It might even allow you to work on some other part of the code for a change! It’s important to share the knowledge, but don’t go overboard adding people to your code review for knowledge’s sake. When reviewers see many names on a code review, they may assume that other developers have “got this” and that they don’t need to invest as much time and effort into the review. Perhaps they won’t even do the review at all. In general, two to five reviewers should be enough for most changes. Key habits:- Get your code reviewed by feature experts and stakeholders.
- Use your code review to familiarize feature novices.
- Don’t assign your code reviews to too many people.
As code reviews expose developers to new ideas and technologies, they write better and better code.
6. Guide your reviewers
It’s easy to get lost in someone else’s code. Your reviewers will not have the same intimate familiarity with the whys and hows of your changes. In order to help them along, be sure to leave comments on your code reviews. Tell people where to start, or explain why a particular change was made the way it was. Consider leaving these kinds of comments in your code (is that heretical to suggest?) to help future readers as well. If your code review tool supports it, don’t be afraid to provide multimedia comments to guide people along. Images or short videos showing your changes can provide a great deal of helpful context. Don’t be afraid to pull reviewers to your computer to discuss your changes in person if the pull request warrants it. However, do try to be respectful of their time when doing so. Key habits:- Leave helpful comments on code reviews and/or in the code to show flow of thought.
- Provide background for the changes and testing tips for the code that can be reviewed by running it.
- Explain complex algorithms.
- Use multimedia to your advantage.
- Review complex or large changes in person.
Provide explanatory comments in your pull requests to help reviewers know what you were thinking when you made changes.
When done right, code reviews actually save time in the long run.
Remember to give the kinds of code reviews that you would like to review yourself. As you strive to make your pull requests more reviewer-friendly, you will naturally get better reviews and feedback as a result! Continue on to Part 2: Tips for the reviewer.
About Lucid
Lucid Software is a pioneer and leader in visual collaboration dedicated to helping teams build the future. With its products—Lucidchart, Lucidspark, and Lucidscale—teams are supported from ideation to execution and are empowered to align around a shared vision, clarify complexity, and collaborate visually, no matter where they are. Lucid is proud to serve top businesses around the world, including customers such as Google, GE, and NBC Universal, and 99% of the Fortune 500. Lucid partners with industry leaders, including Google, Atlassian, and Microsoft. Since its founding, Lucid has received numerous awards for its products, business, and workplace culture. For more information, visit lucid.co.