[webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to

[Oliver Hunt]

I don't understand why anyone is _speculatively_ rolling out patches.

You should only be rolling it out if you _know_ the patch is bad.

That said if you do rollout a random unrelated patch it is obviously your job to roll it back in.  You can't say "i thought this broke something, but i was wrong.  Here you can have that bug again."  There is no case where the original author needs to be involved as we've already determined that they did nothing wrong - the original breakage (of whatever form) was not caused by the patch you selected randomly, and they were not the author responsible for landing anything (eg. the rollout).

--Oliver

On Dec 11, 2012, at 1:21 PM, Peter Kasting <pkasting at chromium.org> wrote:

> On Tue, Dec 11, 2012 at 1:19 PM, Emil A Eklund <eae at chromium.org> wrote:
> > I don't understand your logic.  A patch landed, the sheriff thinks maybe it
> > was bad and rolls it out, then it turns out it was a red herring.  Why is it
> > not now the sheriff's responsibility to re-land?  After all, the patch was
> > landed originally by people who understood it and hasn't been seen to cause
> > any problems.
> 
> There might very well have been other changes that conflicts with it.
> If it applies cleanly then I agree with you that whoever rolled it out
> should reland it. If there are conflicts or if it requires merging in
> any way though I'd argue that the original author needs to get
> involved.
> 
> There are certainly cases where the original author needs to be involved, but I'd be happy just saying this is a judgment call.  Usually rollouts happen not long after a patch lands, and roll-ins happen not long after that.  In those cases, most merge failures are trivial and mechanical and can easily be handled by a conscientious sheriff who reads the relevant changes involved in the conflicts.  Sometimes, of course, that's not true.  But sheriffs should be biased towards "try to leave working patches in the tree".
> 
> PK
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20121211/08180440/attachment.html>