As the manager of the sheriffs, I am in favour of this proposal. The reasons why are as follow (and to note there are only 3 paid sheriffs to try cover the world):
* A number of r+ with nits land up in the sheriffs queue for checkin-needed. This then puts the onus on the sheriffs, not the reviewer that the right thing has been done. The sheriffs do no have the context knowledge of the patch, never mind the knowledge of the system being changed. * The throughput of patches into the trees is only going to increase. If there are failures, and the sheriffs need to back out, this can be a long process depending on the failure leading to pile ups of broken patches. A number of people have complained that using autoland doesn't allow us to fail forward on patches. While that is true, there is the ability to do T-shaped try pushes to make sure that you at least compile on all platforms. This can easily done from Mozreview (Note: I am not suggesting we move to mozreview). Regarding burden on reviewers, the comments in this thread just highlight how broken our current process is by having to flag individual people for reviews. This leads to the a handful of people doing 50%+ of reviews on the code. We, at least visibly, don't do enough to encourage new committers that would lighten the load to allow the re-review if there are nits. Also, we need to do work to automate the removal of nits to limit the amount of re-reviews that reviewer can do. We should try mitigate the security problem and fix our nit problem instead of bashing that we can't handle re-reviews because of nits. David On 9 March 2017 at 21:53, Mike Connor <mcon...@mozilla.com> 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 > > _______________________________________________ > firefox-dev mailing list > firefox-...@mozilla.org > https://mail.mozilla.org/listinfo/firefox-dev > > _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
