On Sat, Aug 1, 2009 at 11:45 AM, Ojan Vafai <ojan@chromium.org> wrote:
On Sat, Aug 1, 2009 at 2:08 PM, Adam Barth <abarth@webkit.org> wrote:
On Sat, Aug 1, 2009 at 11:04 AM, David Kilzer<ddkilzer@webkit.org> wrote:
> Bugzilla has the ability to create additional 4-state flags at both the attachment level and at the bug level.  (Note that bugs.webkit.org does not have bug-level flags enabled.)
>
> For example, we could create a "commit" attachment flag which would have four states (just like the "review" flag):  <none>, commit?, commit+, commit-.
>
> I'm not sure if this helps or hurts, though it would have made abarth's desired workflow much nicer.

Yeah, that sounds great.  Should we try that out for a while and see
if it's useful?

This seems fine to me, except it adds yet another layer of complexity to the review process. I think getting patches landed more quickly is probably worth it though. This is essentially a slightly more complicated (but easier to implement?) version of Maciej's proposal from a few weeks ago to get rid of r* and have the following four review states:

REQUESTED
DENIED
APPROVED WITH MODIFICATIONS
APPROVED

It seems to me that the only difference is APPROVED becomes READY TO LAND (or LANDABLE or something like that).


Btw, I see one downside to a commit queue:  When you manually commit something, you're supposed to watch the build bots for breakage.  If the submit queue is running all the tests on all the platforms then it doesn't really matter, but if not there'll need to be automated rollbacks when things break on the waterfall build bots.  This could easily become complicated if the commit queue can submit multiple patches before the waterfall bots show green for the previous patch.

I guess the point that I'm trying to make is that tests should either be run on every buildbot configuration, there needs to be some pretty robust rollback code, or the queue needs to let everything cycle green before committing the next patch.  The last suggestion seems like a good start.

Note also that, if the system rolls back a patch and the bots don't go back to their previous state, the submit bot needs to stop so someone can take a look manually (so things don't get further hosed).

One benefit over the above is that, I don't think we need to restrict commit+ to people with svn commit bit, as long as a patch is r+'ed or r+'ed with modifications.

Ojan 

_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev