Mozreview

There is a strong argument in modern software engineering that a sequence of smaller changes is preferable to a single large change. This approach facilitates development (easier to debug, quicker to land), testing (less functionality to verify at once), reviews (less code to keep in the reviewer’s head), and archaeology (annotations are easier to follow). Recommended limits are in the realm of 300-400 changed lines of code per patch (see, for example, the great article “How to Do Code Reviews Like a Human (Part Two)”)....

In 2017, Engineering Productivity is starting on a new project that we’re calling “Conduit”, which will improve the automation around submitting, testing, reviewing, and landing commits. In many ways, Conduit is an evolution and course correction of the work on MozReview we’ve done in the last couple years. Before I get into what Conduit is exactly, I want to first clarify that the MozReview team has not really been working on a review tool per se, aside from some customizations requested by users (line support for inline diff comments)....

In Q3 the MozReview team started to focus on tackling various usability issues. We started off with a targetted effort on the “Finish Review” dialog, which was not only visually unappealing but difficult to use. The talented David Walsh compressed the nearly full-screen dialog into a dropdown expanded from the draft banner, and he changed button names to clarify their purpose. We have some ideas for further improvements as well....

A great post on code review is making its rounds. It’s started some discussion amongst Mozillians, and it got me thinking about how MozReview helps with the author’s points. It’s particularly interesting because apparently Twitter uses Review Board for code reviews, which is a core element of the whole MozReview system. The author notes that it’s very important for reviewers to know what reviews are waiting on them, but also that Review Board itself doesn’t do a good job of this....

A few weeks ago, mdoglio found an article from six years ago comparing Review Board and Splinter in the context of GNOME engineering. This was a fascinating read because, without having read this article in advance, the MozReview team ended implementing almost everything the author talked about. Firstly, I admit the comparison isn’t quite fair when you replace bugzilla.gnome.org with bugzilla.mozilla.org. GNOME doesn’t use attachment flags, which BMO relies heavily on....

As mentioned in my previous post on MozReview, one of the biggest sources of confusion is the way we present the “squashed” diffs, that is, the diff that show all of the changes in a commit series, the sum of all the proposed changes. We also refer to these as “parent” review requests, since they function as something to hold all the commits together. They are stored in MozReview as separate review requests, similar to the individual commits....

MozReview was intentionally released early, with a fairly minimal feature set, and some ugly things bolted onto a packaged code-review tool. The code-review process at Mozilla hasn’t changed much since the project began—Splinter, a slightly fancier UI than dealing with raw diffs, notwithstanding. We knew this would be a controversial subject, with a variety of (invariably strong) opinions. But we also knew that we couldn’t make meaningful progress on a number of long-desired features, like autolanding commits and automatic code analysis, without moving to a modern repository-based review system....

Two weeks ago the MozReview developers got together to do some focussed hacking. It was a great week, we got a lot of stuff done, and we clarified our priorities for the coming months. We deployed several new features and improvements during the week, and we made good progress on several other goals. For this week, we actually opted to not go to a Mozilla office and instead rented an AirBNB in Montreal—our own little hacker house!...

MozReview will soon be using Bugzilla’s new OAuth-like1 API keys and auth delegation. This is long overdue, and, in addition to providing security benefits, will eliminate all those confusing session-expired errors (e.g. bug 1178814). After we deploy the change, all users will need to log back into MozReview’s Review Board2 instance. This time, rather than entering your Bugzilla credentials directly into Review Board, when you go to the “Log In” page, you’ll be redirected to BMO....