[webkit-reviews] review granted: [Bug 47650] Make --port a global option and pass the port information to the commit-queue subprocess : [Attachment 70708] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Oct 13 21:48:17 PDT 2010
Eric Seidel <eric at webkit.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 47650: Make --port a global option and pass the port information to the
commit-queue subprocess
https://bugs.webkit.org/show_bug.cgi?id=47650
Attachment 70708: Patch
https://bugs.webkit.org/attachment.cgi?id=70708&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=70708&action=review
Relatively disgusting. BUt not all your fault. Please fix the above nits.
> WebKitTools/Scripts/webkitpy/tool/commands/queues.py:130
> - self.options = options # FIXME: This code is wrong.
Command.options is a list, this assumes an Options element!
> + self._options = options
Please dont' remove this fixme, this code is still WRONG.
> WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py:87
> + queue._options = Mock()
> + queue._options.iterations = 3
Um. Clearly wrong.
> WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py:83
> + def assert_queue_outputs(self, queue, args=None, work_item=None,
expected_stdout=None, expected_stderr=None, expected_exceptions=None,
options=None, tool=MockTool()):
setting tool=MockTool() is wrong here. That will share an instance of
MockTool() for all callers!
More information about the webkit-reviews
mailing list