Re: The future of commit access policy for core Firefox

On Mon, Mar 13, 2017 at 1:24 PM, Lawrence Mandel <[email protected]>
wrote:

> One issue with r+ with nits that we ran into last year is that the
> resulting patch/diff is often committed directly to the repo and not
> uploaded back to Bugzilla or MozReview. This makes it difficult to audit
> the changes to the repo. Keeping the review system in sync with what lands
> (regardless of the review requirements) will make it easier to automate a
> repo audit and reduce the time that our reviewers need to spend looking at
> code changes in the audit scenario. Any concerns with making it a
> requirement that the final patch/diff is documented in the bug/review tool
> rather than landing directly?
>

Submitting the final patches to two places instead of one seems like
busywork to me, and I don't do it (even though some do). I don't know what
it buys us, given that pulsebot posts the hashes of the pushed changes in
the bug.

So I would object to this.


>
> Lawrence
>
> On Mon, Mar 13, 2017 at 4:01 PM, Eric Rescorla <[email protected]> wrote:
>
> > On Mon, Mar 13, 2017 at 12:30 PM, Boris Zbarsky <[email protected]>
> wrote:
> >
> > > On 3/13/17 1:33 PM, Eric Rescorla wrote:
> > >
> > >> 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>.
> > >>
> > >
> > > This seems entirely too plausible.  :(
> > >
> > > 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.
> > >>
> > >
> > > This seems reasonable, if that's the goal.  But this is not the goal
> > > mconnor had in his original post.  I'd love to get to the point where
> we
> > > agree on the goals.
> >
> >
> > Me too. I have come to the conclusion that mconnor's goal cannot be
> > achieved
> > without substantial disruption.
> >
> >
> > - 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 doesn't quite address your "r+ without nits, then the patch author
> > > updates it anyway" scenario; presumably we would need something to
> > address
> > > that too.
> >
> >
> > Sorry, I should have clarified this. In any case, the final (landed)
> patch
> > must be
> > either reviewed or posted by someone trusted. The nits thing is a side
> > issue
> > (which you would think I would have realized from earlier in my own
> > message!)
> >
> > -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
> >
> _______________________________________________
> dev-planning mailing list
> [email protected]
> https://lists.mozilla.org/listinfo/dev-planning
>