On Fri, Jul 26, 2013 at 9:36 PM, Brad Lassey <blas...@mozilla.com> wrote:
> On 7/26/13 9:30 AM, Ehsan Akhgari wrote: > >> 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. >> > + 1 > I also think a commit hook is a bit of overkill I also agree that a commit hook seems like overkill. I have found the build system team to be very responsive and helpful regarding my more involved build system change review requests, so in general I agree with the idea of the proposed policy given the current state of things, though I think it is worded more strictly than necessary. For example, it is OK to make something build conditionally based on a flag when previously it was always built. Similarly, if I'm just adding a new subdirectory of code or tests or whatever to an existing module, or re-arranging the files across directories in a module, it is hardly worth anybody's time to get a build system peer to review it; if even that is so prone to being problematic, then that is a bug in the build system that should be corrected. I think there is something more important than publishing a policy that you could do: publish the guidelines for modifying the build system. I.e. document the things that you'd say in a code review so that if/when I ask you for a review, we're not rehashing stuff you have told 1,000 people 1,000 times. Cheers, Brian _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
