Pre-Commit Code Reviews

Posted by

If there’s a method of code review that creates some anxiety and confusion, it’s the pre-commit code review. Some development teams have embraced it and built large, complex software such as Linux and Git with email-based pre-commit reviews.

For most teams though, pre-commit review causes too many disruptions in a highly disrupted world. It’s unfortunate because pre-commit reviews allows for early feedback and smaller commits — both tenets of code quality. Is there a way to make pre-commit reviews less of a distraction?

What exactly is Pre-Commit code review?

There are different ways to perform a pre-commit code review, but the definition remains mostly the same regardless:

Pre-commit review is the process of going through a changeset, often in the form of a diff, to find defects before the code is commited to a long-living branch.

There are a few arguments working in favour of pre-commit reviews. It encourages making smaller commits so that there is less code to review at a time. A smaller change is likely to have a smaller impact on the system as a whole. It also becomes easier to identify where and when a bug was introduced. Smaller, more frequent commits are also friendly to Continuous Delivery and Continuous Deployment pipelines, which are being embraced by many organizations to provide value to their customers on a non-stop basis.

Pre-commit reviews help get timely feedback on the work that you’ve accomplished. Because the code needs to be reviewed before it gets commited, you’ll be motivated to find someone who’s willing to review the code sooner rather than later so that it can get into a user’s hands. This peer assessment is a great way to learn new techniques from a fellow developer.

Last, but certainly not least, a pre-commit review helps catch bad designs and errors early on in the development cycle. This can be especially useful when on-boarding new members who aren’t familiar with the team’s development standards. It’s much easier to ask a developer to re-work a small part of their code than completly changing the approach when the feature is finished.

Detractors of the technique dislike the high volume of context switching that occurs with pre-commit code review. On average, I probably commit once every hour or two, depending on what I’m working on. This means that every hour I need to find someone to review my code. If I’m working on a team with four other developers, they’ll also need reviews to be done at a minimum once to twice per day. That’s a lot of lost time due to context switching.

Ways to do pre-commit reviews

You may already be doing a pre-commit review without even realizing it. Whenever you are about to do a commit, take a few minutes to review each file you changed for any improvements by comparing it with the previous version of the file. This is a simple and easy way to improve your own code. Sure, you’re not reviewing it with anyone else, but it is a first step towards improving the quality of the code that gets checked in. Even if you don’t succeed in getting anyone else onboard with pre-commit code review, this is an easy way to reap some of the benefits of it yourself.

The next step towards moving to a pre-commit review process is to sit down with a colleague and do a complete peer review. You should review a diff of each file involved, explain the changes you made and look for implementation details that might be wrong. Talking about your code out loud might make you realize that you’ve gone wrong in certain spots. It can be fairly stressful to show your code to someone in this way, so make sure to put your best foot forward by first doing your own personal review.

The previous two paragraphs have been about doing your review before you commit. As strange as it may sound, there are variations on pre-commit review that allow you to do it after you’ve commited your code. In this scenario, pre-commit means before it’s commited to a long living branch (such as develop or master). A common approach to put this in practice would be to have many short-lived branches (think less than a day) with a handful of commits each. At that point, you can use your favorite code review tool to review, comment, and address defects. Once all issues are resolved, merge into your long living branch and move on.

What about doing pre-commit reviews in a DVCS environment, such as Git? Should a pre-commit review be done before commiting to your local repository or before pushing to the remote? Do it before you perform a local commit. It’s easier to keep track of what changed and it keeps you honest by making sure you don’t skip some commits.

While I haven’t worked with it extensively, it seems that Phabricator is built to handle both pre- and post-commit workflows. Look for a review of it in the coming months on this blog.

Should you give it a go?

Definitely. Even if it never goes beyond doing your personal review before commiting, you’ll still be contributing to keeping the code clean and complete. I didn’t go into the details of all the pros and cons of pre and post review, but I hope you got the sense of what each can bring to the table. And as I teased in the open of this post, there’s no one right answer. Do what feels right for you and your team!

Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s