[Webkit-unassigned] [Bug 169308] Add support for Bindings EWS

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=169308

--- Comment #6 from Aakash Jain <aakash_jain at apple.com> ---
Comment on attachment 303745
  --> https://bugs.webkit.org/attachment.cgi?id=303745
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=303745&action=review

overall looks good to me. Few comments inline.

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:223
> +class MockBindingsEarlyWarningSystem(AbstractEarlyWarningSystem):

The filename is jscews_unittest.py, should either rename the file or move the binding test code to a new file.

> Tools/Scripts/webkitpy/tool/bot/jscews_unittest.py:260
> +        with self.assertRaises(ScriptError):

why do we need this? If it is required, shouldn't it be required in all the similar failure test cases below (e.g.: test_ineffective_patch)?

> Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py:194
> +        if hasattr(self._delegate, 'group') and self._delegate.should_build:

do we need to check for 'group' here. 
Also does all the delegates have should_build? I think it should be fine, but please verify for CommitQueueTaskDelegate and StyleQueueTaskDelegate.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170309/2dae891b/attachment.html>