From:  Mark Côté <mcote@mozilla.com>
Date:  10 Mar 2017 09:55:42 Hong Kong Time
Newsgroup:  news.mozilla.org/mozilla.dev.planning
Subject:  

Re: The future of commit access policy for core Firefox

NNTP-Posting-Host:  107.190.34.123

On 2017-03-09 8:19 PM, Bobby Holley wrote:
> On Thu, Mar 9, 2017 at 5:08 PM, Mark Côté  wrote:
>
>> A mistake we made with MozReview was rolling out a new review tool at the
>> same time as the new push-based/autoland automation and workflow. This had
>> a number of repercussions, including requiring users to learn and use a new
>> review tool in order to benefit from autoland and the commit-series
>> approach, but also making it difficult to prioritize fixes and improvements
>> to the various pieces of the system.
>>
>> Having recognized this, we're decoupling this automation and hooking it up
>> to BMO, so we can concentrate on getting it right, since most people are
>> used to using BMO to review patches (this is Project Conduit that I've
>> blogged and posted about in the mozilla.tools newsgroup).
>
>
> This sounds like the right approach.
>
> That said, there can still be hitches in the landing process that are not
> necessarily related to the review tool (recall all the edge cases that bz
> hit a few weeks ago). Decoupling from the review interface is a good first
> step, but building a replacement for {git,hg} push that satisfies
> everyone's use-cases is equally hard.

Indeed, it is.  First, I don't know that Conduit, or any tool, can 
satisfy *everyone's* use cases.  But we're certainly going to aim for 
the common ones.

The other point I was trying to get across is that by decoupling the 
automation, we can focus on it and not split our time between that and 
improvements to (and sometimes fixes for) a code-review tool at the same 
time.  Also the way we're developing the new system, as separate, small 
services, will make the turnaround time on improvements much faster 
(working within Review Board's extension system was a terrible idea for 
something like this).


Mark


>
> We've also grown the team, which was previously understaffed for the size
>> of the project.  We should have some initial pieces ready to demo in a few
>> weeks, but we won't officially launch it until it's good and ready.
>>
>
> That's great to hear.
>
>
>>
>> (Having a richer and more powerful code-review tool is also still a goal,
>> but we'll deal with that later.)
>>
>> Mark
>>
>>
>> On 2017-03-09 5:59 PM, Bobby Holley wrote:
>>
>>> At a high level, I think the goals here are good.
>>>
>>> However, the tooling here needs to be top-notch for this to work, and the
>>> standard approach of flipping on an MVP and dealing with the rest in
>>> followup bugs isn't going to be acceptable for something so critical to
>>> our
>>> productivity as landing code. The only reason more developers aren't
>>> complaining about the MozReview+autoland workflow right now is that it's
>>> optional.
>>>
>>> The busiest and most-productive developers (ehsan, bz, dbaron, etc) tend
>>> not to pay attention to new workflows because they have too much else on
>>> their plate. The onus needs to be on the team deploying this to have the
>>> highest-volume committers using the new system happily and voluntarily
>>> before it becomes mandatory. That probably means that the team should not
>>> have any deadline-oriented incentives to ship it before it's ready.
>>>
>>> Also, getting rid of "r+ with comments" is a non-starter.
>>>
>>> bholley
>>>
>>>
>>> On Thu, Mar 9, 2017 at 1:53 PM, 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
>>>> _______________________________________________
>>>> dev-planning mailing list
>>>> dev-planning@lists.mozilla.org
>>>> https://lists.mozilla.org/listinfo/dev-planning
>>>>
>>>>
>> _______________________________________________
>> dev-planning mailing list
>> dev-planning@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-planning
>>