The proposal sounds good to me but I guess you wouldn't want to be notified of every small addition/change to Makefiles in test directories? I suppose you're targeting actual changes to dependencies etc, but where do we draw the line?
Can we maybe add a push hook intelligent enough to separate actual changes from minor changes that don't need build peer review by scanning for certain keywords? - Tim On 07/18/2013 02:00 AM, 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 > -- Tim Taubert Firefox Engineer ttaub...@mozilla.com _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
