Phabricator

Phabricator at Mozilla

Mozilla runs a Phabricator instance at https://phabricator.services.mozilla.com/, mainly for code review. It is currently the preferred solution for code review. MozReview was decommissioned in 2018Q3, and Splinter has been decommissioned in 2019.

Also see Lando, an automatic code-landing system integrated with Phabricator.

Phabricator and Lando are maintained by the Engineering Workflow team.

Documentation

User documentation for Mozilla's Phabricator instance is available on Read the Docs. There is also a user FAQ for Phabricator and Lando.

Operations and Testing

• Upgrade process
• Test plan

Background

Why the change?

Code review is a critical part of the Firefox engineering process but hasn’t changed much since Mozilla’s early days. Our home-developed tools have worked well for us but are increasingly dependent upon scarce resources to maintain, build and customize. Using our own tool, something that isn’t used by other organizations, also makes onboarding new people, staff and volunteers much more time-consuming than it needs to be.

We are confident the short-term/immediate costs incurred—standing up a new tool, having to shift expectations and refactor workflows—will be more than paid back with surplus time and energy available for process automation (and later, with faster onboarding).

Why did we choose Differential?

Differential, originally developed at Facebook and currently used by many organizations, both open- and closed-source, has been trialled by several teams at Mozilla over the last few months. Although no code review tool is perfect, we believe it addresses several of the deficiencies in Review Board and is better suited to the Firefox engineering process. It is also an opportunity to decouple our automation pieces in order to focus on them after Differential’s launch, where we believe we will have a greater impact to Firefox development.

What will go out in the initial launch?

1. Deployment of a central Differential (code review) installation, supported by CloudOps, along with supporting tools like Diffusion (repository browser) and Herald (event-driven notifications). We will continue to use our Bugzilla instance, bugzilla.mozilla.org (aka BMO), for issue tracking for the foreseeable future.
2. Lightweight integration of Differential with BMO. This will include
   • BMO integration, as discussed above.
   • Integration with the Autoland service, which is currently used for around 50% of commits to mozilla-central and an integral part of the Quantum Stylo project.

What will happen to Splinter and MozReview?

After a short trial period, MozReview will be shut down, and Splinter will be turned off for the main Firefox products on BMO: Core, Firefox, and Toolkit. Contribution guides and documentation will be updated to refer to Phabricator for code-change submissions. The exact archival procedures are will be figured out soon.

Will I be able to request customizations to Differential?

We plan to limit customizations, but not to zero. We’re making this change (to using a third-party tool) to reduce the support burden on internal teams. The fewer customizations we make the more that happens. Any customizations will generally use Phabricator’s Conduit API; extensions will be limited to changes that are not possible to implement via the API. We have no intention of forking any Phabricator tools.

How will I get patches/commits up for review?

To begin with, we will keep to the general Phabricator workflow, including use of the Arcanist command-line tool, although we will likely provide easy installation and some abstraction via mach and MozillaBuild.

After initial launch, we will build support for "push to review", that is, a commit-based system, similar to MozReview's approach.

Where will the flags for feedback/review/ui-review be?

We’re going with the review flow built into Differential, which does not support multiple types of flags as Bugzilla’s attachment-flag system does. However, the actions that a reviewer can perform in Differential are more straightforward: rather than setting “r+” or “r-”, Differential’s options include “resign as reviewer”, “request changes”, and “accept revision”, which roughly map to clearing a r? flag, setting r-, and setting r+, respectively. Differential also distinguishes between a reviewer and a “blocking reviewer”, which could be seen as requesting feedback versus requesting review. Finally, compared to Bugzilla, Differential has a clearer indication of the current state of the reviews on a revision and what needs to happen.

We’re using the opportunity presented by switching to a new code review tool to try out this simpler approach, which will particularly benefit new contributors. This is possibly the biggest change in process, and we know it may take some time to get used to.

Will I be able to push to review?

Most likely, some time after initial launch.

A commit-based system offers many advantages over a patch-based system, mostly because of the metadata, including history, preserved in commits. The ability to use hg/git’s “push” command to submit patches for review was also one of the much-appreciated aspects of MozReview. However, it’s a tricky thing to get right. We never solved the problem of confidential patch review in MozReview, since mapping Bugzilla’s security groups to the Mercurial review repository is rather difficult. Our support for micro-/stacked commits was also a bit more confusing than we liked.

In the interests of doing a staggered launch and iterative development, and so users can start to get used to Differential as soon as possible, the initial launch will only support the standard Phabricator methods for submitting changes, that is, arc (and via the web UI, although that is less convenient and powerful). We will build a commit-based system around Phabricator soon after.

Will there be alternative code review tools available?

No single tool is going to make everyone happy, but we don’t plan to support multiple tools.

What will the team do if they won’t be working on the tool itself?

In addition to (re)building a commit-based system, we have plenty of things lined up already. Here are a few:

• Linting upon patch submission.
• Fully automated landings. Signal your intention to land a patch when you submit it, and after it gets reviewed by authorized developers Autoland will land it automatically.
• Automatic conflict detection, indicating if a patch can’t be cleanly applied to the tree before you try to land it, or even before it’s reviewed.
• Automatic, intelligent Try runs based on what parts of the code were modified.
• Tracking a patch through its lifecycle: review, landing, merges, uplifts, and any backouts along the way.

The pattern here is that these are automated analyses and actions that are part of a pipeline from patch submission to review to landing to release. These are the steps in the engineering process that are largely unique to Firefox’s complex and highly scaled nature. Our team believes that the biggest positive impact to developer productivity lies in this area.

As noted above, these enhancements are either dependent on or greatly simplified by a commit-based model, which will we (re)build first.

We are we rebuilding parts of MozReview around Phabricator?

MozReview started out as an experiment and prototype that simultaneously delivered a new code review tool with some process automation. A lot of this automation was built into Review Board extensions and, later, into a custom fork. We also customized the tool to mimic the BMO patch review process as closely as we could.

Making major changes to Review Board and tightly coupling process automation to it resulted in a heavily constrained development environment and increasing maintenance burdens. Last year we came to the realization that a major architectural change and a refocusing of priorities were overdue.

At the same time we took a hard look at Review Board and concluded that, even without the added complexities of our customizations, it had some problems and deficiencies that were rather difficult to fix, including loading times, inaccuracies and omissions in diffs, and basic UI structure. Taking a look at alternative code-review solutions, of which none are perfect, it seems that Differential better meets Firefox engineering’s needs and already has some supporters at Mozilla.