New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 720034 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task



Sign in to add a comment

WebUI: Use PRESUBMIT ESLint checks to minimize trivial stylistic review comments.

Project Member Reported by dpa...@chromium.org, May 9 2017

Issue description

Background
In WebUI, we used closure_linter in the past, which was removed at [1] for good
reasons. Since then, clang-format is gradually being used across the JS codebase
to enforce consistent formatting of the code. While clang-format is very useful,
it does not perform any checks for issues that can't be auto-formatted.

Proposal
I am proposing using ESLint in a complementary way to clang-format, to perform
additional checks on the code (see attached screenshot for example error report).

Expected benefits
 - Consistency across JS cobebase in Chromium.
 - Increase automatic enforcement of JS styleguide as much as possible.
 - PRESUBMIT integration will save a lot of code reviewers and reviewees time.

Why ESLint?
ESlint seems to be the best open source available tool for JS linting.
 - Active development.
 - Used by many popular projects (among others Node.js [2] and Chromium's DevTools [3]).
 - Highly customizable (200+ rules, see http://eslint.org/docs/rules/)
 - Understands latest ES6 syntax.
 - Flexible rule enforcement (can override rules when necessary, either at a
   folder level or even for a few lines in the code).
 - Rules/features are being considered with popular JS style-guides in mind 
   (namely Google's and AirBnB's style guides, which is very convenient).

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=567770#c28
[2] https://github.com/nodejs/node/blob/master/.eslintrc.yaml
[3] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/.eslintrc.js

 
Cc: rbpotter@chromium.org
Status update, next stepsz;

1) Request third_party review for adding ESLint to node_modules dependencies, https://codereview.chromium.org/2867663004.
2) Fix lint errors across chrome/browser/resources/ sub-folders
3) Enable ESLint integration with PRESUBMIT scripts, example https://codereview.chromium.org/2872703003.
4) Expand scope to include code under https://cs.chromium.org/chromium/src/ui/webui/resources/ and https://cs.chromium.org/chromium/src/chrome/test/data/webui/.

Added example output attachment.
example_output.png
74.6 KB View Download

Comment 4 by dpa...@chromium.org, May 22 2017

Cc: michae...@chromium.org
ESLint binary has been added to WebUI's dependencies!

So the next step is basically to determine the best strategy of rolling out PRESUBMIT checks. Describing some approaches below.

Approach 1: Gradual roll-out of both ESLint rules and folders covered by PRESUBMIT.
 - Start with a minimal ESLint configuration (basically enable just a handful of rules)
 - Add PRESUBMIT checks for chrome/browser/resources first (roughly as done at https://codereview.chromium.org/2872703003).
 - Start adding more ESLint rules, per Google and Chromium styleguides. This entails fixing
   all existing violations before turning on a rule.
 - Once we feel that we reached a good set of rules being enforced, expand the scope of the PRESBUMIT checks to gradually include other folders like 

chrome/browser/resources/
ui/webui/resources
components/about_ui
components/flags_ui
components/version_ui
chrome/test/data


Approach 2: Roll out PRESUBMIT to all JS folders at once. Iterate only on the ESLint rules.

- Add PRESUBMIT checks somewhere within https://cs.chromium.org/chromium/src/tools/web_dev_style/js_checker.py, which covers all foldres mentioned above (except chrome/test/data).
- Start adding more ESLint rules, per Google and Chromium styleguides. This entails fixing all existing violations before turning on a rule.


I am leaning towards approach #1, because I think it allows for faster iteration (less files to fix, less reviewers, less developers affected by PRESUBMIT checks), until we feel more comfortable rolling out changes to all of Chromium's JS codebase.

Thoughts?

Comment 5 by dbeam@chromium.org, May 22 2017

i vote #2 because it's weird to have multiple sources of JS style errors show up (you'd get 2 messages in the console when trying to upload)

Comment 6 by dpa...@chromium.org, May 23 2017

Can we get the best of both approaches?

Meaning, that we add the new checks as a new ESLintCheck() function within [1], but maintain a white-list of folders,initially only containing chrome/browser/resources/, where the new ESLintCheck() will be applied? Then, gradually expand that white-list and eventually completely remove it? 

[1] https://cs.chromium.org/chromium/src/tools/web_dev_style/js_checker.py
Labels: -Type-Feature Type-Task
i vote #2 because we'll have more data on the impact of enabling an ESLint rule if we immediately surface all relevant errors. if we limit the initial errors & fixes to c/b/resources, we're more likely to be caught off guard by the scope of the "crush syndrome" of handling all the rules we've enabled in other directories.


as a suggestion, i also support deferring or ignoring certain directories:

* non-MD UIs: we'll probably rewrite these to some extent, at some point. the benefit of getting ESLint to pass on version_ui, flags_ui, etc. might not outweigh the cost of fixing decade-old code with significant style differences.

* chrome/test/data: again, some of it is quite old. even the new stuff, since it's not closure-compiled, is likely to have shortcuts and messes that might not be worth the effort to clean up.


also, tentatively changing Type: Feature => Type: Task (this is not a UI feature/improvement. maybe if the component is Infra...)

Comment 8 by dpa...@chromium.org, May 23 2017

Ok, I started working on approach 2 as per comments #5 and #7.

Comment 9 by dpa...@chromium.org, May 23 2017

Status update:

 - ESLint PRESUBMIT checks integrated with tools/web_dev_style/js_checker.py are implemented at [1]. As requested by michaelpg@ in the design-doc, I am starting with a dummy ESLint configuration where only a single rule is enabled (no-extra-semi).
 - A CL that fixes all violations of no-extra-semi rule across all folders where js_checker.py is already being used is at [2].

[1] https://codereview.chromium.org/2872703003
[2] https://codereview.chromium.org/2902033002

Moving forward, I think the same pattern makes sense for adding new rules. Specifically:
 step 1: Ensure that the rule is either in the Google styleguide already, or in our own Chromium styleguide, or gather consensus to add it to our styleguide.
 step 2: Send a CL that fixes all violations of the rule in existing code.
 step 3: Update .eslintrc.js configuration file to enable the checks for everyone.

Besides adding new rules in our ESLint configuration, I will look into possibly replacing some of our existing RegEx based checks with ESLint checks (example candidate is the GetElementByIdCheck at https://cs.chromium.org/chromium/src/tools/web_dev_style/js_checker.py?l=55)
Cc: calamity@chromium.org tsergeant@chromium.org
Project Member

Comment 11 by bugdroid1@chromium.org, May 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9de1000c0a7625e951a5a80303ab63d24b67542c

commit 9de1000c0a7625e951a5a80303ab63d24b67542c
Author: dpapad <dpapad@chromium.org>
Date: Wed May 24 17:45:05 2017

WebUI: Fix violations of no-extra-semi lint rule.

This is in preparation of enabling http://eslint.org/docs/rules/no-extra-semi
check during PRESUBMIT.

BUG=720034
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2902033002
Cr-Commit-Position: refs/heads/master@{#474342}

[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/chromevox/chromevox/injected/api.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/chromevox/chromevox/injected/keyboard_handler.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/chromevox/common/dom_util.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/login/screen_supervised_user_creation.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/switch_access/auto_scan_manager.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/switch_access/automation_manager.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/switch_access/automation_predicate.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/switch_access/keyboard_handler.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/switch_access/options.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/switch_access/prefs.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/switch_access/switch_access.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/switch_access/switch_access_interface.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/switch_access/test_support.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/chromeos/switch_access/tree_walker.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/options/geolocation_options.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/options/language_list.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/options/startup_overlay.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/options/supervised_user_create_confirm.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/options/supervised_user_import.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/options/supervised_user_learn_more.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/options/supervised_user_list_data.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/settings/prefs/prefs.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/chrome/browser/resources/settings/site_settings/cookie_tree_node.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/ui/webui/resources/js/cr/ui/alert_overlay.js
[modify] https://crrev.com/9de1000c0a7625e951a5a80303ab63d24b67542c/ui/webui/resources/js/webui_resource_test.js

Project Member

Comment 12 by bugdroid1@chromium.org, May 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/061530882425512579d47b110cd07fc2fc8dc43c

commit 061530882425512579d47b110cd07fc2fc8dc43c
Author: dpapad <dpapad@chromium.org>
Date: Thu May 25 05:42:36 2017

WebUI: Fix more violations of no-extra-semi lint rule.

Missed those on the original crrev.com/2902033002.

This is in preparation of enabling http://eslint.org/docs/rules/no-extra-semi
check during PRESUBMIT.

BUG=720034
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2900253006
Cr-Commit-Position: refs/heads/master@{#474569}

[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/about_nacl.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/bookmark_manager/js/bmm.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/bookmark_manager/js/main.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/chromeos/arc_support/background.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/cryptotoken/gnubby.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/cryptotoken/hidgnubbydevice.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/cryptotoken/sha256.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/cryptotoken/usbgnubbydevice.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/engagement/site_engagement.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/extensions/extensions.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/feedback/js/event_handler.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/gaia_auth_host/post_message_channel.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/hotword/nacl_manager.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/hotword/page_audio_manager.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/md_bookmarks/actions.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/md_bookmarks/reducers.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/md_bookmarks/types.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/pdf/browser_api.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/pdf/gesture_detector.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/pdf/main.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/pdf/zoom_manager.js
[modify] https://crrev.com/061530882425512579d47b110cd07fc2fc8dc43c/chrome/browser/resources/quota_internals/event_handler.js

Project Member

Comment 13 by bugdroid1@chromium.org, May 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ff82d51d2b29b40486761fb60a2521af5f73e3cb

commit ff82d51d2b29b40486761fb60a2521af5f73e3cb
Author: dpapad <dpapad@chromium.org>
Date: Thu May 25 18:40:15 2017

WebUI: Hook up ESLint to web_dev_style PRESUBMIT.

Using a trivial .eslintrc.js config for now, just so that the checks
can be enabled. More lint rules will be added in follow up CLs.

BUG=720034
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2872703003
Cr-Commit-Position: refs/heads/master@{#474720}

[add] https://crrev.com/ff82d51d2b29b40486761fb60a2521af5f73e3cb/.eslintrc.js
[modify] https://crrev.com/ff82d51d2b29b40486761fb60a2521af5f73e3cb/OWNERS
[modify] https://crrev.com/ff82d51d2b29b40486761fb60a2521af5f73e3cb/tools/web_dev_style/OWNERS
[modify] https://crrev.com/ff82d51d2b29b40486761fb60a2521af5f73e3cb/tools/web_dev_style/js_checker.py

Project Member

Comment 15 by bugdroid1@chromium.org, May 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e511b111bdebc1cdb7588d9c22e034ab70715b9

commit 1e511b111bdebc1cdb7588d9c22e034ab70715b9
Author: dpapad <dpapad@chromium.org>
Date: Wed May 31 03:31:26 2017

WebUI: Enable ESLint rule for missing semicolons.

 - Fixing almost all existing violations
   (remaining ones are fixed at crrev.com/2903063007).
 - Enabling http://eslint.org/docs/rules/semi check during PRESUBMIT

This enforces the following section from the styleguide:
https://google.github.io/styleguide/jsguide.html#formatting-semicolons-are-required

BUG=720034
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2909503003
Cr-Commit-Position: refs/heads/master@{#475771}

[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/.eslintrc.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/bluetooth_internals/expandable_list.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/bluetooth_internals/value_control.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/bookmark_manager/js/main.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/chromevox/braille/braille_table.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/chromevox/braille/expanding_braille_translator.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/chromevox/chromevox/injected/event_suspender.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/chromevox/common/media_widget.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/chromevox/common/spannable.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/chromevox/testing/callback_helper.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/login/login_shared.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/login/md_login_shared.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/login/oobe.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/login/oobe_welcome.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/power.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/chromeos/switch_access/automation_predicate.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/cleanup_tool/cleanup_browser_proxy.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/cryptotoken/gnubbies.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/inspect/inspect.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/md_bookmarks/dnd_manager.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/net_internals/modules_view.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/sync_file_system_internals/dump_database.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/sync_file_system_internals/extension_statuses.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/sync_file_system_internals/sync_service.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/resources/sync_file_system_internals/task_log.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/chrome/browser/ui/webui/options/certificate_manager_browsertest.js
[modify] https://crrev.com/1e511b111bdebc1cdb7588d9c22e034ab70715b9/ui/webui/resources/js/cr/ui/array_data_model.js

Project Member

Comment 16 by bugdroid1@chromium.org, May 31 2017

Project Member

Comment 17 by bugdroid1@chromium.org, May 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c9c24ec6d1d8ddaae83a9118c5824b6c40f9854

commit 5c9c24ec6d1d8ddaae83a9118c5824b6c40f9854
Author: dpapad <dpapad@chromium.org>
Date: Wed May 31 20:51:34 2017

js_checker.py: Restore smoke tests for linting violations.

Restoring test to ensure that document.getElementById() calls are flagged as
style violations, now that these checks are performed via ESLint. These tests
are not meant to be exhaustive, instead they just validate that our PRESUBMIT
integration with ESLint works.

BUG=720034

Review-Url: https://codereview.chromium.org/2917473002
Cr-Commit-Position: refs/heads/master@{#476014}

[modify] https://crrev.com/5c9c24ec6d1d8ddaae83a9118c5824b6c40f9854/PRESUBMIT_test_mocks.py
[modify] https://crrev.com/5c9c24ec6d1d8ddaae83a9118c5824b6c40f9854/tools/web_dev_style/js_checker.py
[add] https://crrev.com/5c9c24ec6d1d8ddaae83a9118c5824b6c40f9854/tools/web_dev_style/js_checker_eslint_test.py

Project Member

Comment 19 by bugdroid1@chromium.org, Jun 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4bcb1a848a99744f64f48301f5f797fb09b30e2c

commit 4bcb1a848a99744f64f48301f5f797fb09b30e2c
Author: dpapad <dpapad@chromium.org>
Date: Tue Jun 06 21:41:02 2017

WebUI: Fix/suppress some existing violations of no-restricted-globals.

Adding suppressions for cases where util.js document.getElementById can't
be used because the code runs on an extension or content_script, or just
because a dependency to util.js is unwanted.

For remaining cases, actually replacing document.getElementById() with $().

BUG=720034
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2921783003
Cr-Commit-Position: refs/heads/master@{#477419}

[add] https://crrev.com/4bcb1a848a99744f64f48301f5f797fb09b30e2c/chrome/browser/resources/chromeos/chromevox/.eslintrc.js
[modify] https://crrev.com/4bcb1a848a99744f64f48301f5f797fb09b30e2c/chrome/browser/resources/chromeos/keyboard/keyboard_utils.js
[add] https://crrev.com/4bcb1a848a99744f64f48301f5f797fb09b30e2c/chrome/browser/resources/chromeos/select_to_speak/.eslintrc.js
[modify] https://crrev.com/4bcb1a848a99744f64f48301f5f797fb09b30e2c/chrome/browser/resources/chromeos/switch_access/options.js
[modify] https://crrev.com/4bcb1a848a99744f64f48301f5f797fb09b30e2c/chrome/browser/resources/hotword/audio_client.js
[modify] https://crrev.com/4bcb1a848a99744f64f48301f5f797fb09b30e2c/chrome/browser/resources/instant/instant.js
[modify] https://crrev.com/4bcb1a848a99744f64f48301f5f797fb09b30e2c/chrome/browser/resources/print_preview/print_preview_animations.js
[modify] https://crrev.com/4bcb1a848a99744f64f48301f5f797fb09b30e2c/chrome/browser/resources/print_preview/settings/other_options_settings.js
[modify] https://crrev.com/4bcb1a848a99744f64f48301f5f797fb09b30e2c/ui/webui/resources/js/cr.js
[modify] https://crrev.com/4bcb1a848a99744f64f48301f5f797fb09b30e2c/ui/webui/resources/js/util.js

Project Member

Comment 20 by bugdroid1@chromium.org, Nov 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9

commit ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9
Author: dpapad <dpapad@chromium.org>
Date: Mon Nov 13 19:44:13 2017

WebUI: Fix some lint errors in chrome/test/data/webui.

This is in preparation of enabling ESLint checks on that folder.

Bug: 720034
Change-Id: I1fcb208f7e0c94b2f8457c55a5399ae863dfe951
Reviewed-on: https://chromium-review.googlesource.com/764386
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516009}
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/cr_elements/cr_action_menu_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/cr_elements/cr_dialog_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/cr_elements/cr_policy_indicator_behavior_tests.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/cr_elements/cr_scrollable_behavior_tests.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/extensions/cr_extensions_browsertest.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/extensions/extension_detail_view_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/extensions/extension_error_page_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/extensions/extension_test_util.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/extensions/extension_view_manager_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/md_bookmarks/command_manager_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/md_bookmarks/md_bookmarks_focus_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/md_bookmarks/reducers_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/md_bookmarks/test_util.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/md_history/history_metrics_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/md_history/history_routing_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/md_history/test_util.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/md_user_manager/import_supervised_user_tests.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/print_preview/print_preview_destination_search_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/print_preview/print_preview_tests.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/android_apps_page_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/category_default_setting_tests.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/cr_settings_browsertest.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/cr_settings_interactive_ui_tests.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/device_page_tests.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/easy_unlock_browsertest_chromeos.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/fake_language_settings_private.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/google_assistant_page_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/internet_page_tests.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/passwords_and_autofill_fake_data.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/privacy_page_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/quick_unlock_authenticate_browsertest_chromeos.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/route_tests.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/search_settings_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/settings_autofill_section_browsertest.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/settings_toggle_button_tests.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/site_details_permission_tests.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/site_details_tests.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/startup_urls_page_test.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/test_lifetime_browser_proxy.js
[modify] https://crrev.com/ddf12d1ea02dd3fdd60b72bc3feba5bc051c92f9/chrome/test/data/webui/settings/test_site_settings_prefs_browser_proxy.js

Project Member

Comment 21 by bugdroid1@chromium.org, Nov 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/92a111ac2159b171ade5d11382f6a56b3ef2446b

commit 92a111ac2159b171ade5d11382f6a56b3ef2446b
Author: dpapad <dpapad@chromium.org>
Date: Tue Nov 21 20:45:02 2017

WebUI: Run ESLint presubmit checks for chrome/test/data/webui.

Also fixing remaining ESLint violations automatically with --fix flag.

Bug: 720034
Change-Id: I3fe1b0fbe7b559316a26c1526abacbde0486f418
Reviewed-on: https://chromium-review.googlesource.com/775597
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518379}
[add] https://crrev.com/92a111ac2159b171ade5d11382f6a56b3ef2446b/chrome/test/data/webui/.eslintrc.js
[add] https://crrev.com/92a111ac2159b171ade5d11382f6a56b3ef2446b/chrome/test/data/webui/PRESUBMIT.py
[modify] https://crrev.com/92a111ac2159b171ade5d11382f6a56b3ef2446b/chrome/test/data/webui/accessibility_audit_browsertest.js
[modify] https://crrev.com/92a111ac2159b171ade5d11382f6a56b3ef2446b/chrome/test/data/webui/bluetooth_internals_browsertest.js
[modify] https://crrev.com/92a111ac2159b171ade5d11382f6a56b3ef2446b/chrome/test/data/webui/draganddroptoinput.js
[modify] https://crrev.com/92a111ac2159b171ade5d11382f6a56b3ef2446b/chrome/test/data/webui/interventions_internals_browsertest.js
[modify] https://crrev.com/92a111ac2159b171ade5d11382f6a56b3ef2446b/chrome/test/data/webui/sandboxstatus_browsertest.js
[modify] https://crrev.com/92a111ac2159b171ade5d11382f6a56b3ef2446b/chrome/test/data/webui/test_api.js
[modify] https://crrev.com/92a111ac2159b171ade5d11382f6a56b3ef2446b/chrome/test/data/webui/webview_content_script_test.js
[modify] https://crrev.com/92a111ac2159b171ade5d11382f6a56b3ef2446b/tools/web_dev_style/presubmit_support.py

Cc: shik@chromium.org
Project Member

Comment 23 by bugdroid1@chromium.org, Dec 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c679720af6e8a2451705abf1acdcd7fe080c5a6

commit 7c679720af6e8a2451705abf1acdcd7fe080c5a6
Author: dpapad <dpapad@chromium.org>
Date: Tue Dec 11 01:37:03 2018

WebUI: Enable no-var and prefer-const ESLint rules for most folders.

Specifically:
 - Update chrome/browser/resources/.eslintrc.js to have the desired
   ESLint checks.
 - Explicitly override the default checks for folders that still have
   plenty of violations (bluetooth_internals/, chromeos/, cryptotoken/,
   local_ntp/, media_router/)

Bug: 792774,720034
Change-Id: I505b9ce7773bb706c47757447260630a3c4a38b1
Reviewed-on: https://chromium-review.googlesource.com/c/1357531
Reviewed-by: Dan Beam <dbeam@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615370}
[rename] https://crrev.com/7c679720af6e8a2451705abf1acdcd7fe080c5a6/chrome/browser/resources/.eslintrc.js
[rename] https://crrev.com/7c679720af6e8a2451705abf1acdcd7fe080c5a6/chrome/browser/resources/bluetooth_internals/.eslintrc.js
[copy] https://crrev.com/7c679720af6e8a2451705abf1acdcd7fe080c5a6/chrome/browser/resources/chromeos/.eslintrc.js
[modify] https://crrev.com/7c679720af6e8a2451705abf1acdcd7fe080c5a6/chrome/browser/resources/chromeos/chromevox/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/chromeos/multidevice_setup/.eslintrc.js
[modify] https://crrev.com/7c679720af6e8a2451705abf1acdcd7fe080c5a6/chrome/browser/resources/chromeos/select_to_speak/.eslintrc.js
[modify] https://crrev.com/7c679720af6e8a2451705abf1acdcd7fe080c5a6/chrome/browser/resources/chromeos/switch_access/.eslintrc.js
[copy] https://crrev.com/7c679720af6e8a2451705abf1acdcd7fe080c5a6/chrome/browser/resources/cryptotoken/.eslintrc.js
[copy] https://crrev.com/7c679720af6e8a2451705abf1acdcd7fe080c5a6/chrome/browser/resources/local_ntp/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/management/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/md_bookmarks/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/md_downloads/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/md_extensions/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/md_history/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/md_user_manager/.eslintrc.js
[copy] https://crrev.com/7c679720af6e8a2451705abf1acdcd7fe080c5a6/chrome/browser/resources/media_router/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/omnibox/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/pdf/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/print_preview/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/settings/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/snippets_internals/.eslintrc.js
[delete] https://crrev.com/41a1fad4c04e0c753656e710a06d38cae263bcef/chrome/browser/resources/welcome/onboarding_welcome/.eslintrc.js

Sign in to add a comment