EngineeringProductivity/Projects/MozReview

Status

Requirements for a next-generation code-review tool are settled and the design complete. We're currently fixing up a few last rough edges before general deployment; see blockers on bug 1021929.

We are aiming for deployment in mid-August 2014.

Background

The old: patches

The main code-review tool in use at Mozilla is currently (July 2014) Splinter, which is integrated into Bugzilla. Splinter uses a very old approach to code reviews; it merely displays side-by-side, colourized versions of the attached patch file and allows you to add comments to the diff. While tools such as bzexport have improved usability, Splinter is fundamentally limited. Adding useful features like increased context (outside of the patch itself), history, interdiffs, and so on are possible to varying degrees, all are made difficult or largely impossible by the fact that Splinter is based on patches.

The new: repository-centric development

GitHub really popularized the idea of doing repository-centric code review via Git's branches and GitHub's pull requests feature. Essentially, you clone a repository, create a branch, push that branch to a remote, and trigger a code review from that pushed branch.

This approach puts the code repository and version control system front-and-center. Contrast with exchanging patches, which are representations of changes.

There are several advantages to a repository-centric approach to code review:

• You can get full context of the changes from the underlying repo
• You can address review points by pushing a new commit, which inherently allows interdiffing
• You can easily pull down the branch under review for testing

Several other tools, such as Gerrit, implement this functionality as well, but sadly none of the good options support multiple VCSes, and Mozilla still uses Mercurial heavily.

The Solution: Review Board and friends

This is just the basics of the model, with details (including a how-to) to come.

Review Board is an open-source review tool which supports just about every VCS out there. Unfortunately, it is also fundamentally a patch-based system, although it does know about repositories' canonical location so it can grab context that isn't in the diff itself--at least, if the patch still applies. This makes it still a fancy version of Splinter, fundamentally.

Luckily, we have all kinds of clever people here with intimate knowledge of VCSes, including mercurial, so in late winter they banged together a prototype showing that it is possible to convert Review Board to a branch-based system after all!

As an aside, we we say "branches", we mean the git concept of them--lightweight. Branches in mercurial are not the same at all; however, there are two alternatives that provide similar functionality: Mercurial Queues, now considered obsolete by many, and hg bookmarks. Both are supported by our system, though the latter is preferred

Since then, we've settled on a concrete design that makes submitting patches as painless as possible, as well as providing integration to Bugzilla. The main components are

• Review repos. Similar to the Try server, review repos are clones of existing repos, such as mozilla-central.

• Client and server extensions for mercurial that allow bookmarks (and MQs for people who haven't made the switch) to be pushed to the review repos. This is how we preserve history.

• Extensions for Review Board:
  • rbmozui, which has some UI customizations to make the branch-based workflow more obvious (and which will later provide a Mozilla look).
  • rbbz, which integrates Review Board with Bugzilla and does a few other housekeeping tasks with the extra data we're using in our branch-based model.

Bugzilla integration

The rbbz extension ties Review Board to Bugzilla in several ways:

• All authentication, aside from the built-in admin user, is done with Bugzilla.
• All user searching and loading is referred to Bugzilla so that we have up-to-date user info.
• When a review request is published in Review Board, an attachment is created in the corresponding bug. This attachment contains the URL to the review request and automatically redirects when you click on it (as GitHub pull-requests URLs do already).
  • Users entered into the "Target People" field are flagged with r? on the Bugzilla attachment.
• Any comments ("reviews" in Review Board speak) are mirrored (one-way, for sanity) to Bugzilla. As with Splinter, some diff context is preserved along with the comment.
• When someone clicks "Ship it!" (the Review Board equivalent of r+), the associated Bugzilla attachment is r+ed by that user.

There are more plans for Bugzilla integration, but this was deemed the minimum feature set to make the system usable.

The Future: landing reviews

This model lends itself quite well to the work being done on AutoLand and hg transplants. It should be possible to easily, or even automatically, push the reviewed commits onto the target repository, a long-sought-after feature that is extremely tricky, perhaps impossible, with the current patch-based model.