Re: The future of commit access policy for core Firefox

Lawrence Mandel <[email protected]> Wed, 15 Mar 2017 11:10:39 -0400
Newsgroups gmane.comp.mozilla.devel.seamonkey
Message-ID <CAJLuNE178ENSa2y1nxzncF8wNf3ua4x5i0T1J7ivjWEMgvJmtA@mail.gmail.com>
On Wed, Mar 15, 2017 at 10:53 AM, Ehsan Akhgari <[email protected]>
wrote:

> On 2017-03-13 4:38 PM, Mike Connor wrote:
> > On Mon, Mar 13, 2017 at 4:35 PM, Bobby Holley <[email protected]>
> wrote:
> >
> >> 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.
> >
> >
> > I'd agree that posting a bug to two places is a non-starter.  That's a
> > problem we can and should solve with automation. It should be possible to
> > commit to somewhere, have it show up in Bugzilla, and have it land where
> it
> > needs to go.
>
> I feel like I'm starting to reply to every message in this thread with
> "rebases, rebases, rebases".  ;-)
>
> More seriously, what Lawrence is asking for is simply just impossible.
> You can't know what the final patch you will push will be before you
> push it due to the fact that we prohibit pushing more than one head, so
> you may have to do an unlimited number of rebases before you actually
> succeed in landing your patch.
>
> Once your patch has landed, then pulsebot already sends a link to the
> landed commit to the bug, and that is what has to be counted on as the
> actual source of truth as to what needs to landed.  The fact that
> MozReview is incapable of reflecting that in its UI is just a deficiency
> of the review tool which we could fix.  I use Phabricator for
> contributing to LLVM, and there as soon as I manage to push the final
> version of my patch to their SVN repo, Phabricator UI links my review
> page to the real SVN revision landed (example:
> https://reviews.llvm.org/D16761 says
> "Was committed in r260265, but reverted in r260536.")
>

OK. Clearly I didn't quite think that one through. Point taken.

Lawrence