Re: The future of commit access policy for core Firefox

This is something I've been thinking about a bunch over the weekend.  I
think trusted needs a smaller scope than "one of the hundreds of people
with L3 access."  As Bobby noted, the primary issue is the hot paths, so we
should look at how to explicitly define those paths.

While there's some fuzzy lines between modules (and the pan-tree rewrite
case needs a bit more discussion), I think we can quickly to get to a point
where module owners and peers are defined in-tree, in a machine-readable
format that we could use to a) validate that the reviewer(s) of a patch are
the appropriate people and b) suggest potential reviewers.  b) is
especially crucial in terms of better balancing review load, which is
something David Burns noted earlier.  I'm almost certain that we could do a
much better job of balancing review loads.

If we had that, we could reframe this idea as:

* every patch must be reviewed by an appropriate reviewer (verifiable)
* patch authors who are *also* owners/peers or otherwise marked as trusted
for the code in question (membership in SR group, or explicitly trusted by
the module owner) are permitted to carry over a review.
* carrying over a review requires the original patch be marked explicitly
(i.e. review+ and review-with-nits+)

This does require explicitly marking people as trusted, but I don't think
that's a huge burden, since that list of people doesn't change often.

I think this tightens things up, without creating a significant amount of
pain.

-- Mike

On Mon, Mar 13, 2017 at 1:33 PM, Eric Rescorla <[email protected]> wrote:

> On Mon, Mar 13, 2017 at 6:36 AM, Boris Zbarsky <[email protected]> wrote:
>
> > On 3/12/17 9:55 PM, Eric Rescorla wrote:
> >
> >> Alternately, you can create a patch which gets r+ with nits, and
> >> then update with some malicious code and  r=<committer>.
> >>
> >
> > Speaking as a reviewer, for people I don't trust I pretty much never give
> > "r+ with nits", because I don't trust them to address the nits correctly.
> >
>
> Actually, I wish I had written this differently. Say I get an r+ w/o nits,
> I suspect
> that the sheriffs will accept an updated patch (e.g., ostensibly with a
> comment
> fix) that is marked r=<foo>.
>
>
> So, I would ask: do people believe that this is an acceptable state of
> >> affairs or should the minimum number of 'trusted' people required to
> land
> >> a patch be 1
> >> or more?
> >>
> >
> > Obviously the latter.  ;)
>
>
> Me too. And I think "trust" in this case at least arguably should be
> defined as
> "trusted by Mozilla" (e.g., L3 committer). So, one possibility would be
> have a
> policy like the following.
>
> - Every CL must either be written by someone trusted OR r+ed by someone
> trusted.
> - If a patch is r+ with nits, then the final patch must be posted by
> someone
> trusted.
>
> This would ensure that every CL that lands was signed off on in its final
> form
> by a someone trusted.
>
> Does this seem crazy?
>
> -Ekr
>
>
> >
> >
> > -Boris
> >
> >
> > _______________________________________________
> > dev-planning mailing list
> > [email protected]
> > https://lists.mozilla.org/listinfo/dev-planning
> >
> _______________________________________________
> dev-planning mailing list
> [email protected]
> https://lists.mozilla.org/listinfo/dev-planning
>