Cracking the Code Review, Part 1: Preparing Your Code Review
Cracking the Code Review, Part 1: Preparing Your Code Review
In part one of this two part series, MVB Stephen Rollins looks the code review process, and offers a few tips on how to improve upon it.
Join the DZone community and get the full member experience.Join For Free
Code reviews: Some love them, some hate them, but most probably see them as no better than a necessary evil. As a result, we often view them as a mundane task to check off the list–a final gatekeeper before our wonderful features or bug fixes can begin their journey into the wild to be enjoyed by users. However, the code review is a very powerful tool if utilized correctly and can result in substantial benefits for relatively little investment as long as we are willing to invest a bit of time into it.
This is part 1 of a 2-part series on code reviews. Part 2 has tips for the code reviewer.
“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.”
- 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.
- 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 a 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.
- 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.
- 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.
- Leave helpful comments on code reviews and/or in the code to show the 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.
Published at DZone with permission of Stephen Rollins , DZone MVB. See the original article here.
Opinions expressed by DZone contributors are their own.