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