EngineeringProductivity/Projects/MozReview

From MozillaWiki
< EngineeringProductivity‎ | Projects
Revision as of 21:11, 3 August 2016 by Mcote (talk | contribs) (Mcote moved page Auto-tools/Projects/MozReview to EngineeringProductivity/Projects/MozReview: Team name switched a while ago)
Jump to navigation Jump to search

Status

The vast majority of MozReview docs, including current status, how to use, how to contribute, and future plans, are on Read the Docs.

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
  • If you can push commits, you can initiate code review.
  • A well-designed repository-centric approach to code review doesn't need to enforce client-side workflows on people (this matters for Mercurial, where people use a combination of branches, bookmarks, nameless heads, and mq).

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.

Autoland

Autoland is a service to automatically land changes from one repository to another. At the moment, it is configured to land changes from the mozreview gecko repository to try, allowing for try pushes to be created from within the context of a code review.

Several past projects have used the name "Autoland" and have had roughly the same goal. These have mostly failed to achieve deployment due to difficulty in being able to meaningfully determine whether a patch or changeset has "passed" a try run, due to the number of intermittent oranges which are likely to affect a given try push.

These projects include:

In Q4 of 2014, a prototype system was built with the goal of automatically landing changes from try to mozilla-inbound after a successful try run. It used pulse to monitor try runs pushed with a special syntax. Failed jobs were retriggered to determine if the failure was due to an intermittent or not. If successful, the transplant service under development by RelEng was used to transplant the results from try to another repository.

For Q1 of 2015, the goal is to land revisions from the mozreview gecko repository to the try repository. Once this has proven successful, the plan for the immediate future is to be able to land to mozilla-inbound.

Autoland consists of three separate components. One provides a simple REST interface to allow for autoland requests to be made and request status to be queried. Another is a pulse listener which is used to monitor job status. The main autoland service handles landing revisions between repositories, retriggering failed jobs and updating MozReview.

Bugs should be filed in Bugzilla under Tree Management:Autoland.

No results.

0 Total; 0 Open (0%); 0 Resolved (0%); 0 Verified (0%);


Limitations

Do not use the new tool for any confidential bugs. BMO has a rather complex, fine-grained security model, and we need to make sure we can respect that with the new tool. We haven't put in that effort yet, so Review Board will reject any attempt to publish a review request on a confidential bug. In other words, don't use this system for anything that is or could conceivably become confidential. We will announce when the system is ready for confidential patches.