Skip to content
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

tools: enable block-scoped-var eslint rule #27616

Closed
wants to merge 5 commits into from

Conversation

silverwind
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

This enables the block-scoped-var eslint rule and fixes discovered issues. Code that does not rely on the hoisting properties of var is easier to read and understand.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label May 8, 2019
.eslintrc.js Show resolved Hide resolved
@silverwind silverwind added the tools Issues and PRs related to the tools directory. label May 8, 2019
lib/internal/util.js Outdated Show resolved Hide resolved
@@ -10,8 +10,9 @@ const storedUnicode = Object.create(null);
const useDomains = process.env.NODE_USE_DOMAINS;

// Set up one global domain.
let domain;

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Left a nit/question/suggestion, but looks good to me with or without it.

@silverwind
Copy link
Contributor Author

Landed in 55b80f9.

@silverwind silverwind closed this May 10, 2019
cjihrig pushed a commit to cjihrig/node that referenced this pull request May 10, 2019
PR-URL: nodejs#27616
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request May 10, 2019
PR-URL: #27616
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@BridgeAR BridgeAR mentioned this pull request May 21, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants