Bug 125049 – Add JavaScript style checker and teach checker.py about .js files

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Brian Burg]

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.

[Brian Burg]

Created attachment 218130 [details]
patch

[Joseph Pecoraro]

Comment on attachment 218130 [details]
patch

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.

[WebKit Commit Bot]

Comment on attachment 218130 [details]
patch

Rejecting attachment 218130 [details] from commit-queue.

[e-mail] 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.

[Brian Burg]

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.

[WebKit Commit Bot]

Comment on attachment 218130 [details]
patch

Clearing flags on attachment: 218130

Committed r159969: <http://trac.webkit.org/changeset/159969>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.