Skip to content

How Gerrit Code Review Changed My Life

June 29, 2010

Gerrit, the code review tool from the Android Open Source Project, came to the forefront of my attention only recently. When Grzegorz Kossakowski joined the Scala+GWT project, he proposed it as the keystone of the project’s workflow.  Grzegorz had a unique perspective; he interned last year with Gerrit’s creator, Shawn Pearce.

I started experimenting with Gerrit, and I confess it was love at first install.  I think I jumped in at a good time; it sounds like some early adopters had challenges.  But for me, at this mature point in Gerrit’s lifecycle:  Run war file.  Get working system.  Register with OpenID.  Start administering and using.

Of course, if you want to do a complicated JEE setup, you can.  But Gerrit trivially runs as its own standalone application with an embedded Jetty and H2 database.  Wow … does this sound familiar?  Minus the Restlet and OSGi guts we love so much, Gerrit is based on most of the same stack as our own internal applications.  Jetty, H2, GWT … shared by VergePay and GoGoEgo.  Soul brothers, we are.

There’s even a hidden gem inside Gerrit: gwtorm, a lightweight persistence library that Shawn and/or his confederates quietly created to support cross-database, ORM-style operations without all the cognitive incompatibilities that arise when trying to use something like Hibernate with GWT.  This little solution reminds me of why we have been reliant on com.solertium.db for so many projects.  There is, I’m convinced, room for a simple POJO based object mapper somewhere in the world.  More on those musings later, though.

Anyway.  Love.  Not just because the architecture made my heart swell with fraternal pride.  Also because I’d really tried over and over again to institute a code review process based on Rietveld or Review Board, and just couldn’t demand it with a straight face.  I admire the GWT guys for having the discipline to religiously develop via an Subversion and Rietveld process, but Carl and I tried it on GoGoEgo and just couldn’t muster the strength.

Subversion itself was probably 90% of our problem.  I feel that my svn-fu is too weak to do branches and merges in the Subversion world.  The primary issue, for me, is that the cost of failure is so incredibly high.  Mistype a command, or guess wrong in a Team Provider dialog box, and you’ve made a semi-permanent ruin of your repository.  I like to think I’m surrounded by a bunch of smart people, and they’re even worse at the svn-fu than I am.  So, the practical outcome is: we don’t branch, we don’t merge.  We just fight over trunk.  There is occasionally blood.

We were fumbling, in Rietveld, towards a model much like Gerrit’s.  But instead of having a Subversion topic branch, a patchset would land in Rietveld and live its own iterative life there, with an unreasonable amount of work by the contributing developers to maintain and post versions of the patch, just because of slight differences between how our Eclipse based process viewed the repository, and how Rietveld viewed it.

Gerrit’s tight coupling with git really reduces all this pain.  First of all, I like git a lot better than Subversion.  I may be alone in that at the office!  … but I’m not alone in the world.  I just like distributed version control in general, and git’s command line interface makes perfect sense to me.  It’s trivially easy to create a topic branch, reasonably easy to rebase changes into a clear patch, and again trivial to push that change into Gerrit.

And then, a miracle happens.  Nothing.

Gerrit is a quality control speed bump.  Between prolific developers, and the truth that is the repository, a speed bump is Good.  Even on my own private project, where nobody’s reviewing my code, by going through Gerrit, I’m guaranteed a chance to visualize my changeset in a nice Web interface and make sure it contains what I think it contains.  I get a chance to read it and see: Did something nefarious happen with newlines?  Did an unintended file get committed by accident?  Did a cat or dog walk across some keys and “changed” a file in a way I didn’t know about?  All three of these things have happened — to me personally — in the past two weeks.  And I spotted the problem in Gerrit before the attendant poop landed in my repository.

In a team environment, code review itself is great.  It can exist on lots of levels.  In my daily work travels, when I say “code review,” the usual assumption is that I mean some level of rigorous proposal and defense, like a financial audit.  For me, really, I almost never mean anything that stringent.  I think a second pair of eyeballs is always good, even if it’s just a quick sanity check.  When I say that all code should be reviewed, I don’t mean it should necessarily be dissected.  Just read by somebody other than the person who wrote it.  Like blogs, am I right?

With good support for useful email notifications that users can control themselves, Gerrit makes it easy for this heads-up, second-pair-of-eyeballs code review to proceed. Since switching to Gerrit on a couple of large projects, and subscribing to scads of notifications:

  • I’ve gotten a much better feel for the pace of development
  • I’ve spotted commits that had impact (either positive or negative) beyond their intended scope
  • I’ve been a lot more useful in ad hoc discussions when questions come up

I’ve also interceded a couple times to -1 patches that would have done something bad, for reasons that were occasionally subtle.  As a result, nothing bad has landed in the Gerrit managed repositories yet.  Suddenly a genuinely trunk-stable development strategy is much more achievable.

I feel revolutionized.  I really do love Gerrit.  The 2.1.3 release removes my last few quibbles with the UI (thanks to the new Review+Submit feature, and branch listings for non-administrators)  A few more big projects remain to convert to this workflow, and I can’t wait to get there.

Advertisements

From → Technology

Leave a Comment

Leave a Reply

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

WordPress.com Logo

You are commenting using your WordPress.com 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 )

Google+ photo

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

Connecting to %s

%d bloggers like this: