At WillowTree, we are often asked about how we perform code reviews and how the feedback process works when developing software. Today, I’d like to share our process and some best practices we follow when conducting code reviews.
The code review process starts with the way we structure our work. In general, we follow the git flow model of branching and completing work. You can read more about git flow here (http://nvie.com/posts/a-successful-git-branching-model/), but simply put, git flow requires that each developer works off of their own feature branch that is derived from a shared development branch. Once a developer is done with a feature or bugfix, they can then merge that code back into the development branch. It is at this point that we run our code reviews.
When a developer finishes their work on a feature branch, they push it up to the central git repository (in our case GitHub). Once pushed, the developer will create a pull request and assign it to another developer to review. Once the reviewer finishes commenting on the code and the developer makes any necessary modifications, then the developer will merge the pull request into the main development branch. As you can see, the process is minimal. For it to stay that way, we follow some best practices to keep reviews as simple and painless as possible.
It may sound trivial, but one of the most important first steps to an effective code review is discussing how the feature should be built out with your development team. Ideally, this is done before writing a line of code. It’s a step that is especially important if you are new to a code base or just starting out in your development career. Seek out feedback from more senior developers or from developers that have worked on the code base before—they may have keen insight into how the feature should be integrated. Doing this also decreases the risk of a code review where you find out functionality for a feature you spent hours writing code for already existed in the code base or you should have hooked into the system in a different way.
Keep it small
The best code reviews are small code reviews. It turns out that most reviews have the most constructive feedback when they are under 400 lines of code (http://nvie.com/posts/a-successful-git-branching-model/). After about 400 lines the complexity becomes too hard to keep in your head and most reviewers will gloss over the details as the review grows in size.
What about large features—how should you handle those? One way is to keep a main feature branch that you create pull requests off of, where each of the sub-feature branches are small subsets of the overall feature. Once the feature and PRs are done, you can carry out a simple merge of that overall branch into the main line of development. Another option is to commit small pieces of the feature but add a flag to keep that feature disabled until it’s complete.
Keep reviews focused
In addition to keeping reviews small, it’s also important to keep them focused. Try and keep your pull requests limited to a single item of work. Don’t intersperse formatting fixes with code fixes, or add a bunch of dependencies when fixing a bug. This makes it much easier for a reviewer to focus on the important functional changes that you’ve made in your code. If dependencies or other formatting are necessary, then do those as a separate review. You can also comment on what’s in the review to make it easier and quicker for a reviewer if all you’re doing is pulling in app assets or third-party dependencies. It’s also a good idea to go through the PR yourself first and to trim any stray print statements or newlines, or formatting issues before passing it onto a reviewer.
Provide helpful feedback
When reviewing code, it’s also important to give helpful feedback. Instead of commenting “this is wrong,” try to formulate feedback that provides possible solutions instead. For example, “I’m not sure this is correct, what if you tried X?” Mentioning new solutions to solve problems can also be incredibly useful. Some of the best reviews I was in involved reviewers that took the time to show me other code I could use to solve the problem I was working on or articles that could help me learn more.
You are not your code
Finally, it’s important to remember that you are not your code. Reviews can be hard with other developers pointing out things that are incorrect or could be improved, but with the right attitude you can get very constructive feedback. Reviewers should also remember to keep a professional tone in reviews and speak to the issues with the code without talking about the developer that wrote the code. Finally, if you reach an impasse on agreeing on a change to the code, there is no better way to work through the conflict than to have the reviewer and developer sit down face to face and hash it out.
I hope that you find these best practices useful in your own code reviews. We’ve found that code reviews are immensely important and a vital part of the development process.