Similar to "r+ with fixes" are cases where a patch (or patch series) requires reviews from a multitude of people. Which means variable delays in getting reviews, during which things may slightly bitrot or be trivially modified based on feedback from other reviewers. Code refactorings are one example of this. Having to re-request review from everyone, perhaps multiple times, would seem pretty painful. :)
>From previous discussions, it sounded like there was a middle ground in still requiring all landings to go through automation (i.e., a strong link between what actually landed and what was posted in the bug), but allow some slop in what was formally reviewed (i.e., a weak link between last review and what's was asked to land.) The automation might at least require that you can't land a "r=sparky" unless "sparky" did at some point grant r+. At the very least, this makes it somewhat easier to compare what landed with what was last reviewed (with current tools, thanks to ReviewBoard's revision diffing). I suppose this policy also implies no more "Typo fix, no bug" landings. But I never really liked those anyway, so I'm fine with that. Bugs are cheap! :) Justin On Thu, Mar 9, 2017 at 3:11 PM, <chris.ryan.pea...@gmail.com> wrote: > On Friday, March 10, 2017 at 10:53:50 AM UTC+13, Mike Connor wrote: > > (please direct followups to dev-planning, cross-posting to governance, > > firefox-dev, dev-platform) > > > > > > Nearly 19 years after the creation of the Mozilla Project, commit access > > remains essentially the same as it has always been. We've evolved the > > vouching process a number of times, CVS has long since been replaced by > > Mercurial & others, and we've taken some positive steps in terms of > > securing the commit process. And yet we've never touched the core idea > of > > granting developers direct commit access to our most important > > repositories. After a large number of discussions since taking ownership > > over commit policy, I believe it is time for Mozilla to change that > > practice. > > > > Before I get into the meat of the current proposal, I would like to > outline > > a set of key goals for any change we make. These goals have been > informed > > by a set of stakeholders from across the project including the > engineering, > > security, release and QA teams. It's inevitable that any significant > > change will disrupt longstanding workflows. As a result, it is critical > > that we are all aligned on the goals of the change. > > > > > > I've identified the following goals as critical for a responsible commit > > access policy: > > > > > > - Compromising a single individual's credentials must not be > sufficient > > to land malicious code into our products. > > - Two-factor auth must be a requirement for all users approving or > > pushing a change. > > - The change that gets pushed must be the same change that was > approved. > > - Broken commits must be rejected automatically as a part of the > commit > > process. > > > > > > In order to achieve these goals, I propose that we commit to making the > > following changes to all Firefox product repositories: > > > > > > - Direct commit access to repositories will be strictly limited to > > sheriffs and a subset of release engineering. > > - Any direct commits by these individuals will be limited to fixing > > bustage that automation misses and handling branch merges. > > - All other changes will go through an autoland-based workflow. > > - Developers commit to a staging repository, with scripting that > > connects the changeset to a Bugzilla attachment, and integrates > > with review > > flags. > > - Reviewers and any other approvers interact with the changeset as > > today (including ReviewBoard if preferred), with Bugzilla flags as > the > > canonical source of truth. > > - Upon approval, the changeset will be pushed into autoland. > > - If the push is successful, the change is merged to > mozilla-central, > > and the bug updated. > > > > I know this is a major change in practice from how we currently operate, > > and my ask is that we work together to understand the impact and > concerns. > > If you find yourself disagreeing with the goals, let's have that > discussion > > instead of arguing about the solution. If you agree with the goals, but > > not the solution, I'd love to hear alternative ideas for how we can > achieve > > the outcomes outlined above. > > > > -- Mike > > > How will uplifts work? Can only sheriffs land them? > > This would do-away with "r+ with comments addressed". Reviewers typically > only say this for patches submitted by people they trust. So removing "r+ > with comments" would mean reviewers would need to re-review code, causing > an extra delay and extra reviewing load. Is there some way we can keep the > "r+ with comments addressed" behaviour for trusted contributors? > > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
