Teach check-webkit-style how to accept a list of files to diff on the command line
Created attachment 72561 [details] Patch
Comment on attachment 72561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=72561&action=review Looks sane otherwise. I believe your understanding of python's None argument convention is mistaken. I feel like we have this argument often. :) > WebKitTools/Scripts/check-webkit-style:118 > + changed_files = paths if options.diff_files else [] I think you want else None here. [] seems wrong. > WebKitTools/Scripts/webkitpy/style_references.py:72 > + def create_patch(self, git_commit, changed_files=[]): You want None here. :)
Comment on attachment 72561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=72561&action=review >> WebKitTools/Scripts/check-webkit-style:118 >> + changed_files = paths if options.diff_files else [] > > I think you want else None here. [] seems wrong. [] seems to be ok here (it's reallocated each time it's run), but Eric is right that the default function param should be None.
Created attachment 72569 [details] Patch for landing
Committed r71064: <http://trac.webkit.org/changeset/71064>
http://trac.webkit.org/changeset/71064 might have broken GTK Linux 64-bit Debug The following tests are not passing: accessibility/aria-activedescendant-crash.html accessibility/aria-checkbox-text.html accessibility/aria-hidden-update.html accessibility/contenteditable-hidden-div.html accessibility/crash-with-noelement-selectbox.html accessibility/crashing-a-tag-in-map.html accessibility/document-attributes.html accessibility/first-letter-text-transform-causes-crash.html accessibility/hang-in-isignored.html accessibility/nested-layout-crash.html accessibility/nochildren-elements.html accessibility/non-data-table-cell-title-ui-element.html accessibility/non-native-image-crash.html accessibility/radio-button-checkbox-size.html accessibility/removed-anonymous-block-child-causes-crash.html accessibility/removed-continuation-element-causes-crash.html accessibility/table-modification-crash.html accessibility/table-nofirstbody.html accessibility/table-notbody.html accessibility/table-with-empty-thead-causes-crash.html
(In reply to comment #3) > (From update of attachment 72561 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=72561&action=review > > >> WebKitTools/Scripts/check-webkit-style:118 > >> + changed_files = paths if options.diff_files else [] > > > > I think you want else None here. [] seems wrong. > > [] seems to be ok here (it's reallocated each time it's run), but Eric is right that the default function param should be None. After this patch, check-webkit-style doesn't work for me. But it works with [] instead of None. $ Tools/Scripts/check-webkit-style Traceback (most recent call last): File "Tools/Scripts/check-webkit-style", line 134, in <module> main() File "Tools/Scripts/check-webkit-style", line 119, in main patch = checkout.create_patch(options.git_commit, changed_files=changed_files) File "/home/oszi/WebKit/Tools/Scripts/webkitpy/style_references.py", line 73, in create_patch return self._scm.create_patch(git_commit, changed_files=changed_files) File "/home/oszi/WebKit/Tools/Scripts/webkitpy/common/checkout/scm.py", line 748, in create_patch return self.run(['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self.merge_base(git_commit), "--"] + changed_files, decode_output=False, cwd=self.checkout_root) TypeError: can only concatenate list (not "NoneType") to list
Comment on attachment 72569 [details] Patch for landing remove cq+, because it was landed
sorry, it was false positive alarm. https://bugs.webkit.org/show_bug.cgi?id=52261 is the culprit.