[webkit-reviews] review granted: [Bug 69484] watchlist: If the style check fails, then the watchlist will not be run. : [Attachment 111078] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted David Levin <levin at chromium.org>'s
request for review:
Bug 69484: watchlist: If the style check fails, then the watchlist will not be
run.
https://bugs.webkit.org/show_bug.cgi?id=69484

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=111078&action=review


I like executive_throws_when_run.  :)

> Tools/Scripts/webkitpy/tool/commands/queues.py:440
> +	   # Raise any exceptions that occurred while running check-style
> +	   if exception:
> +	       raise exception

There's a slightly snazzier way of doing using the "raise" keyword with no
arguments, but this is more than fine.

> Tools/Scripts/webkitpy/tool/mocktool.py:726
> -    def __init__(self, should_log=False, should_throw=False):
> +    def __init__(self, should_log=False, should_throw=False,
should_throw_when_run=set()):

Eric will tell you to use Non for the default paramater value.	The problem is
that the default paramater value is a static and we modify it by accident, all
future callers will get the modified value!

> Tools/Scripts/webkitpy/tool/mocktool.py:729
> +	   self._should_throw_when_run = should_throw_when_run

If you use None, then this line becomes:

self._should_throw_when_run = should_throw_when_run or set()