[webkit-reviews] review granted: [Bug 64185] garden-o-matic should be able to roll out patches : [Attachment 100226] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 64185: garden-o-matic should be able to roll out patches
https://bugs.webkit.org/show_bug.cgi?id=64185

Attachment 100226: Patch
https://bugs.webkit.org/attachment.cgi?id=100226&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
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.