Bug 52636

Summary: check-webkit-style should ignore the Source/WebCore/thirdparty directory
Product: WebKit Reporter: Stephen White <senorblanco>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: abarth, commit-queue, eric, levin
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch levin: commit-queue-

Stephen White
Reported 2011-01-18 10:09:12 PST
Source/WebCore/thirdparty contains third-party code which does not follow WebKit coding style. check-webkit-style should skip it. (See https://bugs.webkit.org/show_bug.cgi?id=52627 for false positives.)
Attachments
Patch (1.28 KB, patch)
2011-01-19 14:34 PST, James Robinson
levin: commit-queue-
David Levin
Comment 1 2011-01-18 13:11:00 PST
I'm happy to r+ a change Tools/Scripts/webkitpy/style/checker.py to make this happen :)
James Robinson
Comment 2 2011-01-19 14:34:24 PST
David Levin
Comment 3 2011-01-19 14:43:27 PST
Comment on attachment 79487 [details] Patch You may want to just make the "-" (which should get rid of all rules if that's what is desired) instead of "-whitespace".
James Robinson
Comment 4 2011-01-19 19:50:12 PST
Comment on attachment 79487 [details] Patch Skipping whitespace is good enough to pass everything for now, but if we start failing other style checks we can add those as well. I'm slightly wary of skipping every single style check just in case some truly insane stuff starts getting added to WebCore/thirdparty/. We can figure it out when we see it :)
WebKit Commit Bot
Comment 5 2011-01-20 00:44:09 PST
Comment on attachment 79487 [details] Patch Rejecting attachment 79487 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'bu..." exit_code: 2 Last 500 characters of output: ands failed: WebCore: CompileC /Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/HTMLScriptElement.o /Projects/CommitQueue/Source/WebCore/html/HTMLScriptElement.cpp normal x86_64 c++ com.apple.compilers.gcc.4_2 CompileC /Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/HTMLSelectElement.o /Projects/CommitQueue/Source/WebCore/html/HTMLSelectElement.cpp normal x86_64 c++ com.apple.compilers.gcc.4_2 (2 failures) Full output: http://queues.webkit.org/results/7561236
Adam Barth
Comment 6 2011-01-20 10:57:48 PST
Comment on attachment 79487 [details] Patch This patch can't have caused a compile error. Very strange.
Eric Seidel (no email)
Comment 7 2011-01-20 13:41:34 PST
I think I was running too many bots on the machine: /Projects/CommitQueue/Source/WebCore/html/HTMLScriptElement.cpp -o /Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/HTMLScriptElement.o /Developer/usr/bin/../libexec/gcc/i686-apple-darwin10/4.2.1/as: can't fork a new process to execute: /Developer/usr/bin/../libexec/gcc/darwin/x86_64/as (Resource temporarily unavailable) /Projects/CommitQueue/Source/WebCore/html/HTMLScriptElement.cpp:181: fatal error: error writing to -: Broken pipe compilation terminated.
Eric Seidel (no email)
Comment 8 2011-01-21 02:16:47 PST
====================================================================== ERROR: test_path_rules_specifier (webkitpy.style.checker_unittest.GlobalVariablesTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Projects/CommitQueue/Tools/Scripts/webkitpy/style/checker_unittest.py", line 197, in test_path_rules_specifier validate_filter_rules(path_rules, self._all_categories()) File "/Projects/CommitQueue/Tools/Scripts/webkitpy/style/filter.py", line 45, in validate_filter_rules "must start with + or -." % rule) ValueError: Invalid filter rule "w": every rule must start with + or -. ---------------------------------------------------------------------- Ran 804 tests in 10.609s
David Levin
Comment 9 2011-01-21 05:50:38 PST
Comment on attachment 79487 [details] Patch You need a comma on line 165 after the ] or else python will interpret the () as a grouping instead of a list since there is only one item in the list.
Darin Adler
Comment 10 2011-06-18 12:06:32 PDT
Comment on attachment 79487 [details] Patch Clearing review flag on this old patch that was rejected by commit-queue.
Note You need to log in before you can comment on or make changes to this bug.