[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