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

lib: use const where possible #26679

Closed
wants to merge 1 commit into from
Closed

lib: use const where possible #26679

wants to merge 1 commit into from

Conversation

JungMinu
Copy link
Member

use const instead of var where possible

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

use const instead of var where possible
@nodejs-github-bot nodejs-github-bot added dgram Issues and PRs related to the dgram subsystem / UDP. net Issues and PRs related to the net subsystem. repl Issues and PRs related to the REPL subsystem. labels Mar 15, 2019
@Trott
Copy link
Member

Trott commented Mar 15, 2019

We usually reject PRs for the lib directory that are exclusively var -> const like this one. A recent previous example is #26504. See the conversation there for some of the reasoning.

@Trott
Copy link
Member

Trott commented Mar 15, 2019

If you want to try to get a var -> const PR for the lib directory accepted, I'd recommend this approach:

  • Configure ESLint to flag instances of var that could be const. (Don't flag instances that would have to be let.)

  • Auto-fix the entire lib directory.

  • Submit the changed lib code along with the ESLint configuration change.

The PR may still get rejected due to churn etc.! But there is at least some support for the idea that we keep getting these kinds of PRs so it may be time to just fix it once and for all. So it might get accepted.

@BridgeAR BridgeAR mentioned this pull request Mar 26, 2019
4 tasks
BridgeAR added a commit to BridgeAR/node that referenced this pull request Mar 28, 2019
targos pushed a commit that referenced this pull request Mar 30, 2019
Refs: #26679

PR-URL: #26915
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
@BridgeAR
Copy link
Member

BridgeAR commented Apr 1, 2019

Superseded by #26915

@BridgeAR BridgeAR closed this Apr 1, 2019
BethGriggs pushed a commit that referenced this pull request Apr 5, 2019
Refs: #26679

PR-URL: #26915
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 8, 2019
Refs: #26679

PR-URL: #26915
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Signed-off-by: Beth Griggs <[email protected]>
@ZYSzys ZYSzys mentioned this pull request Jun 6, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dgram Issues and PRs related to the dgram subsystem / UDP. net Issues and PRs related to the net subsystem. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants