GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — test: convert var->const/let in tests by gibfahn · Pull Request #10685 · nodejs/node

Conversation

@gibfahn

@gibfahn gibfahn commented Jan 8, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Overview

Use eslint to convert all var to const/let in test/, manually fix anything that eslint messed up.

If we're going to go ES6 in test/, we might as well go all the way.

To fix rules with eslint

Apply this:

diff --git a/.eslintrc b/.eslintrc
index cf1f36c86b..e09fd6ac64 100644
--- a/.eslintrc
+++ b/.eslintrc
@@ -110,6 +110,8 @@ rules:
 
   # ECMAScript 6
   # http://eslint.org/docs/rules/#ecmascript-6
+  no-var: error
+  prefer-const: error
   arrow-parens: [2, always]
   arrow-spacing: [2, {before: true, after: true}]
   constructor-super: 2
@@ -119,7 +121,6 @@ rules:
   no-dupe-class-members: 2
   no-new-symbol: 2
   no-this-before-super: 2
-  prefer-const: [2, {ignoreReadBeforeAssign: true}]
   rest-spread-spacing: 2
   template-curly-spacing: 2

Run this:

eslint --fix --rulesdir=tools/eslint-rules "test/**/*.js"
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gibfahn gibfahn added the test Issues and PRs related to Node.js core tests and test infrastructure. label Jan 8, 2017
@nodejs-github-bot nodejs-github-bot added the dont-land-on-v7.x label Jan 8, 2017
@gibfahn

gibfahn commented Jan 8, 2017

Copy link
Copy Markdown
Member Author

cc/ @nodejs/testing

cc/ @silverwind @Trott @not-an-aardvark Is there an easy way to make rules only apply to test/? Also should @not-an-aardvark be on the tools/eslint cc list in onboarding-extras?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos

targos commented Jan 8, 2017

Copy link
Copy Markdown
Member

Is there an easy way to make rules only apply to test/

Add the rule to test/.eslintrc.
gibfahn and not-an-aardvark reacted with thumbs up emoji gibfahn reacted with laugh emoji
All reactions
  • 👍 2 reactions
  • 😄 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot added the dont-land-on-v7.x label Jan 8, 2017
@gibfahn

gibfahn commented Jan 8, 2017

Copy link
Copy Markdown
Member Author

@targos

Add the rule to test/.eslintrc.

Thanks, done
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
cjihrig
cjihrig approved these changes Jan 8, 2017
View reviewed changes

@cjihrig cjihrig left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Rubber stamp LGTM if the CI passes.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gibfahn

gibfahn commented Jan 8, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

CI: https://ci.nodejs.org/job/node-test-commit/7103/

EDIT: CI passed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gibfahn

gibfahn commented Jan 8, 2017

Copy link
Copy Markdown
Member Author

It's probably worth checking the second and third commits:

  • second (manual fixups of eslint issues): 27d10c0
  • third (require let/const in test/): 21725bf
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mscdex mscdex removed the dont-land-on-v7.x label Jan 9, 2017
jasnell
jasnell approved these changes Jan 9, 2017
View reviewed changes

@jasnell jasnell left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Rubber-stamp LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
gibfahn
gibfahn commented Jan 9, 2017
View reviewed changes
Comment thread test/.eslintrc Outdated

@gibfahn gibfahn Jan 9, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Should these be:

no-var: 2
prefer-const: 2

? I'm not sure what the 2 represents here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@targos targos Jan 9, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

2 means error

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gibfahn gibfahn Jan 9, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Changed error to 2 for consistency.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
targos reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@gibfahn
gibfahn force-pushed the var2const branch from 21725bf to cec64bb Compare January 9, 2017 11:08
@nodejs-github-bot nodejs-github-bot added the dont-land-on-v7.x label Jan 9, 2017
@gibfahn

gibfahn commented Jan 9, 2017

Copy link
Copy Markdown
Member Author

CI 2: https://ci.nodejs.org/job/node-test-commit/7112/

EDIT: Still green
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gibfahn
gibfahn force-pushed the var2const branch from cec64bb to 18260b6 Compare January 9, 2017 13:21
@nodejs-github-bot nodejs-github-bot added the dont-land-on-v7.x label Jan 9, 2017
@gibfahn
gibfahn force-pushed the var2const branch from 18260b6 to fabe275 Compare January 10, 2017 22:37
@nodejs-github-bot nodejs-github-bot added the dont-land-on-v7.x label Jan 10, 2017
@gibfahn
gibfahn force-pushed the var2const branch from fabe275 to 5a1242e Compare January 10, 2017 22:45
@nodejs-github-bot nodejs-github-bot added the dont-land-on-v7.x label Jan 10, 2017
@gibfahn

gibfahn commented Jan 10, 2017

Copy link
Copy Markdown
Member Author

CI 3: https://ci.nodejs.org/job/node-test-commit/7158/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gibfahn
gibfahn force-pushed the var2const branch from 5a1242e to 6dfa97d Compare January 10, 2017 22:53
@nodejs-github-bot nodejs-github-bot added the dont-land-on-v7.x label Jan 10, 2017
silverwind
silverwind suggested changes Jan 10, 2017
View reviewed changes
Comment thread test/parallel/test-http-get-pipeline-problem.js Outdated

@silverwind silverwind Jan 10, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think you can const this one.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gibfahn gibfahn Jan 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

➜  node git:(var2const) tools/test.py test/parallel/test-http-get-pipeline-problem.js 
=== release test-http-get-pipeline-problem ===                    
Path: parallel/test-http-get-pipeline-problem
image.length = 45658
/Users/gib/wrk/com/node/test/parallel/test-http-get-pipeline-problem.js:38
  for (const i = 0; i < total; i++) {
                                ^

TypeError: Assignment to constant variable.

If this is what you meant, I don't think so. I can do for (let i = 0; though.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@silverwind silverwind Jan 10, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Weird, I could swear const loop variables once worked, but I see it's failing now. Your let suggestion sounds good.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/parallel/test-http-many-ended-pipelines.js Outdated

@silverwind silverwind Jan 10, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

unneccesary wrapping?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gibfahn gibfahn Jan 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The semicolon puts it over 80 chars 😭

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/parallel/test-http-url.parse-auth-with-header-in-request.js Outdated

@silverwind silverwind Jan 10, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

unneccesary wrapping?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gibfahn gibfahn Jan 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

81 chars again

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@silverwind silverwind Jan 10, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Move the string to a variable so it doesn't look so hideous? :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/parallel/test-net-local-address-port.js Outdated

@silverwind silverwind Jan 10, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

unneccesary wrapping?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gibfahn gibfahn Jan 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

➜  node git:(var2const) ✗ make lint
./node tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules \
	  benchmark lib test tools

/Users/gib/wrk/com/node/test/parallel/test-net-local-address-port.js
  16:1  error  Line 16 exceeds the maximum line length of 80  max-len

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/parallel/test-repl-tab.js Outdated

@silverwind silverwind Jan 10, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

unneccesary wrapping?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gibfahn gibfahn Jan 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

as above

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/parallel/test-vm-static-this.js Outdated

@silverwind silverwind Jan 10, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

unneccesary wrapping?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gibfahn gibfahn Jan 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

as above

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/parallel/test-writeuint.js Outdated

@silverwind silverwind Jan 10, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Same here, can probably use const.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gibfahn gibfahn Jan 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Changed to let as above (const didn't work)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/parallel/test-zlib.js Outdated

@silverwind silverwind Jan 10, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

const again.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/pummel/test-net-connect-econnrefused.js Outdated

@silverwind silverwind Jan 10, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

const here too.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gibfahn gibfahn Jan 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

const doesn't work (the pending++ would be modifying it)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gibfahn gibfahn Jan 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Actually, as is the test failed (it's not run in CI), to fix I had to move the let to the previous line.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
silverwind
silverwind reviewed Jan 10, 2017
View reviewed changes
Comment thread test/.eslintrc Outdated

@silverwind silverwind Jan 10, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Maybe alphabetically sort the rules?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gibfahn reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@gibfahn gibfahn Jan 10, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Will do

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gibfahn
gibfahn force-pushed the var2const branch from 6dfa97d to d478825 Compare January 10, 2017 23:14
@nodejs-github-bot nodejs-github-bot added the dont-land-on-v7.x label Jan 10, 2017
@gibfahn

gibfahn commented Jan 10, 2017

Copy link
Copy Markdown
Member Author

@silverwind Updated, PTAL
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gibfahn

gibfahn commented Jan 10, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Actually, I just realised that the eslint rule isn't showing up all the vars for some reason, I'll investigate later.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@italoacasas italoacasas mentioned this pull request Jan 25, 2017
Proposal to update v7.x-staging #10989
Closed
targos pushed a commit that referenced this pull request Jan 28, 2017
@gibfahn @targos
test: use eslint to fix var->const/let
eda18cf 
Manually fix issues that eslint --fix couldn't do automatically.

PR-URL: #10685
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
targos pushed a commit that referenced this pull request Jan 28, 2017
@gibfahn @targos
tools, test: require const/let in test
06c5604 
PR-URL: #10685
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
v7.5.0 proposal #11062
Merged
@gibfahn
gibfahn restored the var2const branch January 29, 2017 22:30
@gibfahn
gibfahn deleted the var2const branch January 29, 2017 22:32
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
@gibfahn Italo A. Casas
test: use eslint to fix var->const/let
a6bd287 
Manually fix issues that eslint --fix couldn't do automatically.

PR-URL: nodejs#10685
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
@gibfahn Italo A. Casas
tools, test: require const/let in test
31f8f6f 
PR-URL: nodejs#10685
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
@gibfahn Italo A. Casas
test: use eslint to fix var->const/let
61638e6 
Manually fix issues that eslint --fix couldn't do automatically.

PR-URL: nodejs#10685
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
@gibfahn Italo A. Casas
tools, test: require const/let in test
a73ea44 
PR-URL: nodejs#10685
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
@gibfahn gibfahn mentioned this pull request Jan 31, 2017
test: fix test-child-process-flush-stdio on windows (Node V4 only) #10523
Closed
3 tasks done
@jasnell

jasnell commented Mar 8, 2017

Copy link
Copy Markdown
Member

This will need backport PRs in order to land on v6 or v4
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell jasnell added dont-land-on-v4.x labels Mar 8, 2017
@davidben davidben mentioned this pull request Mar 8, 2017
crypto: Remove unused access of tlsext_hostname. #10882
Closed
3 tasks done
@bnoordhuis

bnoordhuis commented Mar 9, 2017

Copy link
Copy Markdown
Member

This is likely preventing a lot of pull requests from back-porting cleanly so I think it should be back-ported with some urgency. Likewise for #10698.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gibfahn
gibfahn restored the var2const branch March 9, 2017 08:28
@gibfahn

gibfahn commented Mar 9, 2017

Copy link
Copy Markdown
Member Author

v4.x backport: #10685
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gibfahn gibfahn mentioned this pull request Mar 9, 2017
v4.x backport - test: convert var->const/let in tests #11773
Closed
2 tasks
@MylesBorins

MylesBorins commented Mar 9, 2017

Copy link
Copy Markdown
Contributor

@gibfahn did can you do a v6.x backport too?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gibfahn

gibfahn commented Mar 9, 2017

Copy link
Copy Markdown
Member Author

@MylesBorins I'll do it once I get the v4.x backport working. There's more pig-wrestling than I'd expected.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gibfahn gibfahn mentioned this pull request Mar 9, 2017
v6.x backport - test: convert var->const/let in tests #11775
Closed
2 tasks
MylesBorins pushed a commit that referenced this pull request Apr 13, 2017
@gibfahn @MylesBorins
tools, test: require const/let in test
daddf5a 
Backport-PR-URL: #11775
PR-URL: #10685
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 13, 2017
@gibfahn @MylesBorins
test: use eslint to fix var->const/let
b2fc0f0 
Manually fix issues that eslint --fix couldn't do automatically.

Backport-PR-URL: #11775
PR-URL: #10685
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 19, 2017
@gibfahn @MylesBorins
tools, test: require const/let in test
de63698 
Backport-PR-URL: #11775
PR-URL: #10685
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 19, 2017
@gibfahn @MylesBorins
test: use eslint to fix var->const/let
db61c95 
Manually fix issues that eslint --fix couldn't do automatically.

Backport-PR-URL: #11775
PR-URL: #10685
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 19, 2017
v6.10.3 proposal #12498
Merged
@gibfahn
gibfahn deleted the var2const branch June 3, 2017 14:34
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
@gibfahn @andrew749
tools, test: require const/let in test
6aea35d 
Backport-PR-URL: nodejs/node#11775
PR-URL: nodejs/node#10685
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
@gibfahn @andrew749
test: use eslint to fix var->const/let
cd87208 
Manually fix issues that eslint --fix couldn't do automatically.

Backport-PR-URL: nodejs/node#11775
PR-URL: nodejs/node#10685
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Trott Trott Trott left review comments
@jasnell jasnell jasnell approved these changes
@cjihrig cjihrig cjihrig approved these changes
+2 more reviewers
@targos targos targos left review comments
@silverwind silverwind silverwind approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

11 participants

@gibfahn @targos @silverwind @italoacasas @jasnell @bnoordhuis @MylesBorins @Trott @cjihrig @mscdex @nodejs-github-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.