On Fri, Sep 25, 2009 at 10:01 AM, Amanda Walker <[email protected]> wrote:
> On Tue, Sep 22, 2009 at 9:06 PM, Dimitri Glazkov <[email protected]>wrote: > >> This all means that we have to be a bit more diligent. We shouldn't be >> paying these unnecessary costs. So, from now on, I propose a fairly >> simple set of new rules: >> >> 1) if you write a Chromium patch for WebKit, you must provide URLs of >> successful trybot runs with your submission. Chromium WebKit reviewers >> will not r+ your patch otherwise. If you can't provide the trybot URLs >> for some reason, please explain in detail why this patch could still >> land. >> >> 2) if the two-sided patch you authored broke the canary and this >> happened with no coordination with the WebKit gardener, you assume >> WebKit gardening responsibility for the next 24 hours. >> >> Hopefully, these amendments to our existing ways will bring a bit more >> peace and stability to the Chromium land. What do you think? >> > > I think they're a good start, but they are symptoms of a larger problem. > "Move fast and clean up messes when they happen" worked great when we had > one big mess a week. These days, we have multiple ones per day. And as > good as the try bots and canaries are (kudos to M-A for setting up the try > bots in the first place, and everyone who helps keep the ever-growing herd > of bots running), they don't catch everything. They don't catch build time > regressions because someone forgot svn:ignores when refactoring where > projects get generated, or accidentally checks something into the wrong > directory (not to pick on anyone in particular, these are just recent > examples). > > I'd add a 3rd principle: > > 3) If you change how chromium is built, however innocuous your change seems > (gyp changes, new libraries, etc.), take extra care and use more reviewers > than usual (preferably including someone intimately acquainted with the > bots, such as maruel, thomasvl, markmentovai, nsylvain, etc.). If you're > reviewing such a change, don't just look at the diffs, try out the patch and > flag anything that seems out of the ordinary. "Build breakage" means more > than just "doesn't compile"; it can also mean "overcompiles" (which kills > bot performance) or "requires a clobber unnecessarily." > I had to land a 2 sided patch yesterday. It blew up an important unit test in a very creative way, and we're still trying to figure out how to clean it up nicely. In the mean time, we have a dev channel release blocker. There has to be a better way... Here's a crazy idea: 4) Create a WebKit.next branch. Have full build bot coverage on it. All integrations would go through it. Other than that, only 2 sided patches would land on it. Whenever everything passes, we'd merge in with the main branch. We'd try very hard never to let it get more than a day or so out of sync. This would make 2 sided merges (which are often the reason for rushing a DEPS roll--don't want to keep the canary red for too long, otherwise it's very hard to sort things out) much less haphazard. We could even have it roll the DEPS automatically every time a new webkit.org patch lands, and thus eliminate our need for dedicated canaries. Yeah, it's crazy....but I kind of like it. And yeah, when the WebKit API lands things should be better in terms of others breaking us, but this addresses 2 sided patches...which are not going away any time soon. J --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
