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

[bugzilla-daemon at webkit.org]

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

Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #304005|review?                     |review+
              Flags|                            |

--- Comment #12 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 304005
  --> https://bugs.webkit.org/attachment.cgi?id=304005
Patch

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

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

Can you use getattr with a default value here?

> Tools/Scripts/webkitpy/tool/bot/retrylogic_unittest.py:1
> +# Copyright (C) 2017 Apple Inc. All rights reserved.

Looks like the file was not moved, but re-added, thus breaking source history. Please re-do the same change, but using a move.

> Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py:94
> +        return EarlyWarningSystemTask(self, patch, self._options.run_tests, self.should_build)

Whenever we use multiple boolean arguments, we should really use named arguments instead.

In WebKit C++ code, we do a similar thing by creating distinct types for boolean arguments, that way it's impossible to mix up the order.

-- 
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/20170310/53f7f613/attachment.html>