Skip to content

Enable the ESLint no-var rule globally #13091

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 13, 2021

Conversation

Snuffleupagus
Copy link
Collaborator

A significant portion of the code-base has now been converted to use let/const, rather than var, hence it should be possible to simply enable the ESLint no-var rule globally.
This way we can ensure that new code won't accidentally use var, and it also removes the need to manually enable the rule in various folders.

Obviously it makes sense to continue the efforts to replace var, but that should probably happen on a file and/or folder basis.

Please note that this patch excludes the following code:

  • The extensions/ folder, since that seemed easiest for now (and I don't know exactly what the support situation is for the Chromium-extension).

  • The entire external/ folder is ignored, since most of it's currently excluded from linting.
    For the code that isn't imported from elsewhere (and should be ignored), we should probably (at some point) bring the code up to the same linting/formatting standard as the rest of the code-base.

  • Various files in the test/ folder are ignored, as necessary, since the way that a lot of this code is loaded will require some care (or perhaps larger re-factoring) when removing var usage.

A significant portion of the code-base has now been converted to use `let`/`const`, rather than `var`, hence it should be possible to simply enable the ESLint `no-var` rule globally.
This way we can ensure that new code won't accidentally use `var`, and it also removes the need to manually enable the rule in various folders.

Obviously it makes sense to continue the efforts to replace `var`, but that should probably happen on a file and/or folder basis.

Please note that this patch excludes the following code:
 - The `extensions/` folder, since that seemed easiest for now (and I don't know exactly what the support situation is for the Chromium-extension).

 - The entire `external/` folder is ignored, since most of it's currently excluded from linting.
   For the code that isn't imported from elsewhere (and should be ignored), we should probably (at some point) bring the code up to the same linting/formatting standard as the rest of the code-base.

 - Various files in the `test/` folder are ignored, as necessary, since the way that a lot of this code is loaded will require some care (or perhaps larger re-factoring) when removing `var` usage.
@Snuffleupagus
Copy link
Collaborator Author

/botio lint

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_lint from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.67.70.0:8877/e02a75d412962d2/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_lint from @Snuffleupagus received. Current queue size: 0

Live output at: http://3.101.106.178:8877/05b7093fd4e1d87/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/e02a75d412962d2/output.txt

Total script time: 2.46 mins

  • Lint: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://3.101.106.178:8877/05b7093fd4e1d87/output.txt

Total script time: 4.11 mins

  • Lint: Passed

@timvandermeij timvandermeij merged commit fb78604 into mozilla:master Mar 13, 2021
@timvandermeij
Copy link
Contributor

Good idea! I fiddled with the test code before and also found that some code should ideally be refactored first to make the var conversion work easier.

@Snuffleupagus Snuffleupagus deleted the eslint-no-var branch March 13, 2021 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants