RESOLVED FIXED 64185
garden-o-matic should be able to roll out patches 
https://bugs.webkit.org/show_bug.cgi?id=64185
Summary garden-o-matic should be able to roll out patches
Adam Barth
Reported 2011-07-08 11:15:39 PDT
We already have a bunch of infrastructure for rolling out patches. The local server can either use the local working copy or ask one of the bots to perform the rollout.
Attachments
work in progress (1.93 KB, patch)
2011-07-09 14:35 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
work in progress (4.57 KB, patch)
2011-07-10 07:59 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (5.93 KB, patch)
2011-07-10 08:45 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2011-07-09 14:35:09 PDT
Created attachment 100218 [details] work in progress
Adam Barth
Comment 2 2011-07-10 07:59:27 PDT
Created attachment 100225 [details] work in progress
Adam Barth
Comment 3 2011-07-10 08:45:23 PDT
Created attachment 100226 [details] Patch
Eric Seidel (no email)
Comment 4 2011-07-10 12:26:50 PDT
Comment on attachment 100226 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100226&action=review I'm not sure I believe in this config object, and I think tool should be held separate. But the change looks good. > Tools/Scripts/webkitpy/tool/commands/gardenomatic.py:38 > + def _prepare_config(self, options, args, tool): > + return { > + 'tool': tool, > + } Odd. I would think we would pass a tool around anyway as its own argument. > Tools/Scripts/webkitpy/tool/servers/gardeningserver_unittest.py:47 > + def __init__(self, server): > + self.server = server pylint is going to get mad that we don't call the original __init__. YOu shoudl call the original __init__ with mocks from this one. > Tools/Scripts/webkitpy/tool/servers/gardeningserver_unittest.py:58 > + def test_empty_state(self): > + expected_stderr = "MOCK run_command: ['echo', 'rollout', '--force-clean', '--non-interactive', '2314', 'MOCK rollout reason']\n" > + self._post_to_path("/rollout?revision=2314&reason=MOCK+rollout+reason", expected_stderr=expected_stderr) Very slick.
Adam Barth
Comment 5 2011-07-10 12:47:58 PDT
Comment on attachment 100226 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100226&action=review >> Tools/Scripts/webkitpy/tool/servers/gardeningserver_unittest.py:47 >> +# The real GardeningHTTPRequestHandler has a constructor that's too hard to >> +# call in a unit test, so we create a subclass that's easier to constrcut. >> +class TestGardeningHTTPRequestHandler(GardeningHTTPRequestHandler): >> + def __init__(self, server): >> + self.server = server > > pylint is going to get mad that we don't call the original __init__. YOu shoudl call the original __init__ with mocks from this one. As the comment says, the entire point of this class is to avoid calling the original __init__.
Adam Barth
Comment 6 2011-07-11 10:51:07 PDT
Comment on attachment 100226 [details] Patch Clearing flags on attachment: 100226 Committed r90762: <http://trac.webkit.org/changeset/90762>
Adam Barth
Comment 7 2011-07-11 10:51:11 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.