Summary: | Add JavaScript style checker and teach checker.py about .js files | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brian Burg <burg> | ||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
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 | ||||||
Attachments: |
|
Description
Brian Burg
2013-12-01 16:35:47 PST
Created attachment 218130 [details]
patch
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. Comment on attachment 218130 [details] patch 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. 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 on attachment 218130 [details] patch Clearing flags on attachment: 218130 Committed r159969: <http://trac.webkit.org/changeset/159969> All reviewed patches have been landed. Closing bug. |