<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body><span class="vcard"><a class="email" href="mailto:ap&#64;webkit.org" title="Alexey Proskuryakov &lt;ap&#64;webkit.org&gt;"> <span class="fn">Alexey Proskuryakov</span></a>
</span> changed
              <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Add support for Bindings EWS"
   href="https://bugs.webkit.org/show_bug.cgi?id=169308">bug 169308</a>
        <br>
             <table border="1" cellspacing="0" cellpadding="8">
          <tr>
            <th>What</th>
            <th>Removed</th>
            <th>Added</th>
          </tr>

         <tr>
           <td style="text-align:right;">Attachment #304005 Flags</td>
           <td>review?
           </td>
           <td>review+
           </td>
         </tr></table>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Add support for Bindings EWS"
   href="https://bugs.webkit.org/show_bug.cgi?id=169308#c12">Comment # 12</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Add support for Bindings EWS"
   href="https://bugs.webkit.org/show_bug.cgi?id=169308">bug 169308</a>
              from <span class="vcard"><a class="email" href="mailto:ap&#64;webkit.org" title="Alexey Proskuryakov &lt;ap&#64;webkit.org&gt;"> <span class="fn">Alexey Proskuryakov</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=304005&amp;action=diff" name="attach_304005" title="Patch">attachment 304005</a> <a href="attachment.cgi?id=304005&amp;action=edit" title="Patch">[details]</a></span>
Patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=304005&amp;action=review">https://bugs.webkit.org/attachment.cgi?id=304005&amp;action=review</a>

<span class="quote">&gt; Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py:194
&gt; +        if not hasattr(self._delegate, 'should_build') or self._delegate.should_build:</span >

Can you use getattr with a default value here?

<span class="quote">&gt; Tools/Scripts/webkitpy/tool/bot/retrylogic_unittest.py:1
&gt; +# Copyright (C) 2017 Apple Inc. All rights reserved.</span >

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

<span class="quote">&gt; Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py:94
&gt; +        return EarlyWarningSystemTask(self, patch, self._options.run_tests, self.should_build)</span >

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.</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>