Code Review (part 1)

I love code review

What is code review? This wikipedia quote sounds ok, but who could love anything that includes "formal process" and "scrutinized"? Sounds like a lot of work, right? What's the upside?

Code review is systematic examination (often as peer review) of computer source code intended to find and fix mistakes
http://en.wikipedia.org/wiki/Code_review

A code review (sometimes called a program inspection) is a formal process where a software developer presents the code he or she has written to other software engineers who are familiar with the project. The code is scrutinized carefully to identify potential bugs, design problems, non-compliance with project standards, inconsistencies, and any other problems in the code.
http://sea.ucar.edu/best-practices/code-review

Code review allows developers to collaborate and improve code by reading it early and catching bugs during development. The earlier bugs are caught, the less impact and expense they cause. Code review is a lot more fun before the changes are live than retroactively trying to figure out what change broke everything in production.

I remember code reviews at my first software company. Someone must have heard they were a good idea, so we had to do code reviews of new features. We waited until the feature was done, then printed out all the code and took a few engineers into a room. We'd sit there for a few hours looking over the printouts before making a few token suggestions and calling it quits. We shared some small insights and caught a few bugs, but overall this heavy process was unstructured, late, disorganized and ineffective. We had the right motivations, but we were looking at code too late in too big of a chunk.

At the other end of the scale is the ad-hoc system of emailing around some diffs or code refs and asking for input. Here the lack of formal process is a pain -- emailing diffs around? Another process flowing through (stalling in) my mailbox? Where do I send my comments, how do I archive the results?

Somewhere in the happy middle are tools for "light weight code review." These tools take a diff and present it in a web interface providing the ability to view the diffs and make comments and enforce some sort of workflow. Gerrit (inspired by Rietveld inspired by Mondrian), Review-Board and BarKeep are some of the open source options, github reviews are free and pay software is available from SmartBear (Code Collaborator), Atlassian (Crucible) among others. These systems all make different trade-offs: pre-vs-post commit, forced-vs-optional reviews, VCS agnostic-vs-integrated, inline vs side-by-side diffs.

At work we've been using Gerrit for two years now after switching from Rietveld when we migrated from SVN to git. Gerrit is very opinionated: it is for mandatory, pre-commit reviews and only supports git. Gerrit integrates nicely with Jenkins continuous integration server for running unit-tests before the review. We originally picked Gerrit at [undisclosed startup] and managed to integrate it into Demand after we were acquired.

For open source projects, I'm happy with github pull-request discussions (I still wish I could see side-by-side diffs! I have this huge monitor for a reason) and couldn't see enforcing the gerrit model (even though the android project does) without discouraging drive-by patches from random developers. But at work, I want the small dollop of process that gerrit provides.

I've been super happy with gerrit and can't wait to tell you more about it in "Code Review, Part II. Gerrit FTW".