Bug 64418

Summary: sheriffbot isn't reopening patches after it lands rollouts
Product: WebKit Reporter: WebKit Review Bot <webkit.review.bot>
Component: New BugsAssignee: Tom Zakrajsek <tomz>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dpranke, eric, ojan, tomz
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

WebKit Review Bot
Reported 2011-07-12 18:20:29 PDT
sheriffbot isn't reopening patches after it lands rollouts Requested by abarth on #webkit.
Attachments
Patch (1.28 KB, patch)
2011-10-21 11:02 PDT, Tom Zakrajsek 		no flags
DetailsFormatted DiffDiff
Patch (2.32 KB, patch)
2012-05-05 23:17 PDT, Tom Zakrajsek 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2011-10-13 16:11:37 PDT
This is worth fixing if it's still the case.
Tom Zakrajsek
Comment 2 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.
Adam Barth
Comment 3 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.
Adam Barth
Comment 4 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.
Tom Zakrajsek
Comment 5 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.
Adam Barth
Comment 6 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.
Tom Zakrajsek
Comment 7 2011-10-21 11:02:31 PDT
Created attachment 111989 [details] Patch
Adam Barth
Comment 8 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.
Tom Zakrajsek
Comment 9 2012-05-05 23:17:46 PDT
Created attachment 140418 [details] Patch
Adam Barth
Comment 10 2012-05-06 09:41:38 PDT
Comment on attachment 140418 [details] Patch Ah, nice approach.
WebKit Review Bot
Comment 11 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>
WebKit Review Bot
Comment 12 2012-05-06 15:32:42 PDT
All reviewed patches have been landed. Closing bug.
Eric Seidel (no email)
Comment 13 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?
Tom Zakrajsek
Comment 14 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.
Note You need to log in before you can comment on or make changes to this bug.