EngineeringProductivity/Projects/MozReview: Difference between revisions

From MozillaWiki
Jump to navigation Jump to search
m (Mcote moved page Auto-tools/Projects/CodeReviewTool to Auto-tools/Projects/MozReview: Adopting "MozReview" as project name)
(MozReview is no more)
 
(9 intermediate revisions by 2 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}}.
 
See also [[Auto-tools/Projects/CodeReviewTool/Deployment|deployment plans]].
 
= 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
* 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 [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.
 
== 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, and similarly if a bug is switched from public to confidential, any associated review requests will '''''automatically''''' be completely deleted.  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.
 
= The Future =
 
== More repos, more VCSes ==
 
We're going to launch with only a couple hg repos, namely mozilla-central and version-control-tools, to make sure everything works properly.  We'll add more hg repos when we verify that everything is solid.
 
In parallel we'll be working on support for git to allow usage for git.mozilla.org and GitHub repos.
 
== 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"