EngineeringProductivity/Projects/MozReview: Difference between revisions

From MozillaWiki
Jump to navigation Jump to search
(MozReview is no more)
 
(16 intermediate revisions by 3 users not shown)
Line 1: Line 1:
= Status =
MozReview has been decommissioned in favour of [https://phabricator.services.mozilla.com Phabricator] and [https://lando.services.mozilla.com Lando].
 
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 [http://hg.mozilla.org/hgcustom/version-control-tools/file/tip/hgext/bzexport 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 [https://code.google.com/p/gerrit/ 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.
 
[http://www.reviewboard.org/ 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: [https://developer.mozilla.org/en-US/docs/Mercurial_Queues Mercurial Queues], now considered obsolete by many, and [http://gregoryszorc.com/blog/2014/06/23/please-stop-using-mq/ 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 [https://wiki.mozilla.org/ReleaseEngineering/TryServer Try server], review repos are clones of existing repos, such as mozilla-central.
 
* Client and server [http://hg.mozilla.org/hgcustom/version-control-tools/file/tip/hgext/reviewboard 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:
** [http://hg.mozilla.org/hgcustom/version-control-tools/file/tip/pylib/rbmozui rbmozui], which has some UI customizations to make the branch-based workflow more obvious (and which will later provide a Mozilla look).
** [http://hg.mozilla.org/hgcustom/version-control-tools/file/tip/pylib/rbbz 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.

Latest revision as of 23:46, 24 October 2018

MozReview has been decommissioned in favour of Phabricator and Lando.

Retrieved from "https://wiki.allizom.org/index.php?title=EngineeringProductivity/Projects/MozReview&oldid=1202877"