Bug 125049

Summary: Add JavaScript style checker and teach checker.py about .js files
Product: WebKit Reporter: Brian Burg <burg>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: commit-queue, glenn, graouts, joepeck, rniwa, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 125045    
Description Flags
patch none

Description Brian Burg 2013-12-01 16:35:47 PST
We should check WebInspectorUI for trivial style problems using check-webkit-style. The first step is to create a JavaScript checker module and associate it with the .js file extension. As this is intended to check the inspector sources, we should defer on checking layout tests, perf tests, and other things written in JavaScript.
Comment 1 Brian Burg 2013-12-01 18:26:04 PST
Created attachment 218130 [details]
Comment 2 Joseph Pecoraro 2013-12-02 13:04:49 PST
Comment on attachment 218130 [details]

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

> Tools/Scripts/webkitpy/style/checker_unittest.py:484
> +        # Check checker attributes on a typical input.
> +        file_base = "foo"
> +        file_extension = "css"
> +        file_path = file_base + "." + file_extension
> +        self.assert_checker_text(file_path)
> +        checker = self.dispatch(file_path)
> +        self.assertEqual(checker.handle_style_error,
> +                          self.mock_handle_style_error)
> +

There is no mention in the ChangeLog of handling .css files as text. It also seems weird to put this test in "test_js_paths" and not a "test_css_paths". Looks fine though.
Comment 3 WebKit Commit Bot 2013-12-02 14:20:13 PST
Comment on attachment 218130 [details]

Rejecting attachment 218130 [details] from commit-queue.

burg@cs.washington.edu does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 4 Brian Burg 2013-12-02 14:21:15 PST
Oops, I don't understand how attachment flags work. Can you set cq+? Also,

(In reply to comment #2)

> There is no mention in the ChangeLog of handling .css files as text. It also seems weird to put this test in "test_js_paths" and not a "test_css_paths". Looks fine though.

This is copied from existing style checkers as a negative test case. IMO, all of the checker constructor unit tests should be deleted since they are pretty useless. But then, I'd get lost deleting a lot of unit tests for lack of purpose.
Comment 5 WebKit Commit Bot 2013-12-02 14:50:29 PST
Comment on attachment 218130 [details]

Clearing flags on attachment: 218130

Committed r159969: <http://trac.webkit.org/changeset/159969>
Comment 6 WebKit Commit Bot 2013-12-02 14:50:31 PST
All reviewed patches have been landed.  Closing bug.