On Thu, Jul 25, 2013 at 5:28 PM, Gregory Szorc <g...@mozilla.com> wrote:
> It appears this proposal was generally well received and I guess that > means it's time to officially enact it. > Thanks for starting this conversation and driving it through, Gregory! > I've written up the review policy at [1] and filed bug 898089 [2] to > enforce/communicate this policy via Mercurial hooks. > While I supported the review policy change here, I'm fairly strongly opposed to the idea of the commit hook enforcing it. I've commented on the bug. Cheers, -- Ehsan <http://ehsanakhgari.org/> > If you experience excessive lag obtaining build peer review, please don't > hesitate to loudly complain. > > [1] > https://wiki.mozilla.org/**Build_config<https://wiki.mozilla.org/Build_config> > [2] > https://bugzilla.mozilla.org/**show_bug.cgi?id=898089<https://bugzilla.mozilla.org/show_bug.cgi?id=898089> > > > On 7/17/13 5:00 PM, Gregory Szorc wrote: > >> Traditionally, it's been very difficult for the build peers to keep on >> top of changes in the build config because changes are occurring on bug >> components we don't follow, are touching files all over the tree, and >> because build peers aren't always asked for review. >> >> The potential for "sneaking" things past build peer review has resulted >> in a number of self-inflicted wounds, the Fennec build rules probably >> being the best example (the dependencies are all wrong and no-op builds >> take ~16s when they should take 1 or 2s). >> >> This history contributed to us implementing a more strict "sandbox" in >> moz.build files: take away as many footguns as possible and there will >> be less self-inflicted wounds. >> >> Unfortunately, the moz.build conversion isn't finished and it will drag >> on for a while. There will still be Makefile.in in the tree and that >> leaves the door open for new badness. >> >> I would like to reinforce the existing policy: *if you are changing a >> Makefile.in, please ask a build peer for review unless the change is >> just adding or removing to an existing list.* >> >> For the most part, people have been abiding by this policy. However, >> things are still creeping through. Unfortunately, some of them wouldn't >> get r+ from a build peer. >> >> Since new Makefile.in badness makes people's lives harder (especially >> when it makes the build slower), I would like to propose a more strict >> policy around Makefile.in changes: *if a non-list change in a >> Makefile.in isn't reviewed by a build peer, it doesn't land or gets >> backed out.* This could potentially be enforced with repository push >> hooks. >> >> I /think/ this proposal is supported by our module governance system >> since Makefile.in are the purview of the build config module. But I >> wanted to run the proposal by people to make sure it is generally >> well-received and there aren't any major concerns. >> >> Gregory >> ______________________________**_________________ >> dev-platform mailing list >> dev-platform@lists.mozilla.org >> https://lists.mozilla.org/**listinfo/dev-platform<https://lists.mozilla.org/listinfo/dev-platform> >> >> > ______________________________**_________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/**listinfo/dev-platform<https://lists.mozilla.org/listinfo/dev-platform> > _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
