Bug 64418 - sheriffbot isn't reopening patches after it lands rollouts
Summary: sheriffbot isn't reopening patches after it lands rollouts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tom Zakrajsek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-12 18:20 PDT by WebKit Review Bot
Modified: 2012-05-06 21:08 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.28 KB, patch)
2011-10-21 11:02 PDT, Tom Zakrajsek
no flags Details | Formatted Diff | Diff
Patch (2.32 KB, patch)
2012-05-05 23:17 PDT, Tom Zakrajsek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description WebKit Review Bot 2011-07-12 18:20:29 PDT
sheriffbot isn't reopening patches after it lands rollouts
Requested by abarth on #webkit.
Comment 1 Adam Barth 2011-10-13 16:11:37 PDT
This is worth fixing if it's still the case.
Comment 2 Tom Zakrajsek 2011-10-19 11:21:21 PDT
Do you mean re-open the bug?  Is this something that webkit-patch create-rollout should do?  I'm having trouble understanding the different use-cases for prepare-rollout, create-rollout, and rollout in webkit-patch.
Comment 3 Adam Barth 2011-10-19 14:44:05 PDT
> prepare-rollout

This is meant to be a local command that only affects the local working copy.  We could rename this to prepare-rollout-locally if that would be clearer.

> create-rollout

This is meant to post a rollout patch to a new bug.  We could rename this to "post-rollout" if that would be clearer.

> rollout

This is meant to actually do a full rollout, including preparing the local change and then landing it.
Comment 4 Adam Barth 2011-10-19 14:44:47 PDT
> Is this something that webkit-patch create-rollout should do?

Sure.  The only risk there is that we'll create a rollout patch but never land it.  I guess that means the person will just have to close their bug again.
Comment 5 Tom Zakrajsek 2011-10-19 14:50:49 PDT
I think that's reasonable.  Especially since the alternative is that there could be a bug out there with a patch waiting to undo an author's change that they might not be aware of.
Comment 6 Adam Barth 2011-10-19 14:53:02 PDT
There's already logic to mark the rollout bug as blocking the original bug.  It's probably easy to make it re-open the bug at that stage.
Comment 7 Tom Zakrajsek 2011-10-21 11:02:31 PDT
Created attachment 111989 [details]
Patch
Comment 8 Adam Barth 2011-10-21 12:50:09 PDT
As I mentioned to tomz in IRC, we can test this either using a step unittest or a command unittest.  In either case, the MockTool will be substituted for the tool and the reopen_bug function on the mock object should generate logging output that we can capture and check.
Comment 9 Tom Zakrajsek 2012-05-05 23:17:46 PDT
Created attachment 140418 [details]
Patch
Comment 10 Adam Barth 2012-05-06 09:41:38 PDT
Comment on attachment 140418 [details]
Patch

Ah, nice approach.
Comment 11 WebKit Review Bot 2012-05-06 15:32:35 PDT
Comment on attachment 140418 [details]
Patch

Clearing flags on attachment: 140418

Committed r116245: <http://trac.webkit.org/changeset/116245>
Comment 12 WebKit Review Bot 2012-05-06 15:32:42 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Eric Seidel (no email) 2012-05-06 15:47:43 PDT
I'm not sure I follow exactly.  This makes it so that when sherriff-bot creates a rollout he re-opens the original bug?
Comment 14 Tom Zakrajsek 2012-05-06 21:08:50 PDT
(In reply to comment #13)
> I'm not sure I follow exactly.  This makes it so that when sherriff-bot creates a rollout he re-opens the original bug?

Yes.