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, test: update ESLint to 2.7.0 #6132

Closed
wants to merge 2 commits into from

Conversation

silverwind
Copy link
Contributor

Checklist
  • tests and code linting passes
  • the commit message follows commit guidelines
Description of change

Updated ESLint from 2.1.0 to 2.7.0 and fixed issues that arose.

Would like some opinions on the prefer-const changes. The rule complains that the variables weren't reassigned and could be made const, but that turned out to be problematic in a few cases where the variable was reassigned from undefined and couldn't be made const in the place of assignment because const doesn't hoist and the variables were accessed earlier.

I'm leaning towards a rule bug in these cases and disabled the rule in the affected tests.

@silverwind silverwind added test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Apr 9, 2016
@@ -1,3 +1,4 @@
/* eslint-disable prefer-const */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here, prefer-const was complaining about server.

@mscdex
Copy link
Contributor

mscdex commented Apr 9, 2016

Couldn't those few tests just be modified to use const instead of let?

@silverwind
Copy link
Contributor Author

Couldn't those few tests just be modified to use const instead of let?

Not without altering behaviour, I think. Take this example:

let foo;

setImmediate(function() {
  console.log(foo);
});

foo = process.hrtime();

Using const, that would be:

const foo = process.hrtime();

setImmediate(function() {
  console.log(foo);
});

foo would be off by one tick, right?

@mscdex
Copy link
Contributor

mscdex commented Apr 9, 2016

For example, I meant change this:

let client;

// ...

server = net.createServer(function(c) {

to this:

// ...

const server = net.createServer(function(c) {

and this:

let before;

// ...

before = process.hrtime();

to this:

// ...

const before = process.hrtime();

@silverwind
Copy link
Contributor Author

Oh, you're right. Looks like your suggestion works and I've been misunderstanding hoisting:

$ node --use-strict -e 'let foo; setImmediate(function() {console.log(foo)}); foo = 1'
1
$ node --use-strict -e 'setImmediate(function() {console.log(bar)}); const bar = 1'
1

@silverwind
Copy link
Contributor Author

Updated with @mscdex's suggestion.

CI: https://ci.nodejs.org/job/node-test-pull-request/2234/

@silverwind
Copy link
Contributor Author

(re-pushed latest commit for the gpg signature)

@silverwind
Copy link
Contributor Author

Looks line one unrelated failure on the CI. Linting took 1min 5sec which seems to be similar to previous version, maybe a tad slower.

@mscdex
Copy link
Contributor

mscdex commented Apr 9, 2016

LGTM

Linting will get faster if #5638 eventually lands ;-)

@Fishrock123
Copy link
Contributor

kinda annoying but LGTM

@Trott
Copy link
Member

Trott commented Apr 10, 2016

This change adds Intl as a valid global from the linter's point of view. \o/ LGTM

@thefourtheye
Copy link
Contributor

LGTM

@silverwind
Copy link
Contributor Author

This change adds Intl

That's not in yet, as it was added in globals@9.4.0, but ESLint has globals@9.2.0.

@Trott
Copy link
Member

Trott commented Apr 10, 2016

That's not in yet, as it was added in globals@9.4.0, but ESLint has globals@9.2.0.

It's in there. ESLint specifies globals 9.2.0 or above (as long as it's 9.x.x). This PR has 9.4.0 installed. CELEBRATE!

@silverwind
Copy link
Contributor Author

@Trott indeed. Today I learned that semver carets also match on minor (thought it was patch only):

> require("semver").satisfies("9.4.0", "^9.2.0")
true

silverwind added a commit that referenced this pull request Apr 10, 2016
PR-URL: #6132
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
silverwind added a commit that referenced this pull request Apr 10, 2016
PR-URL: #6132
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
@silverwind
Copy link
Contributor Author

Thanks for reviews, landed in 2f6ff1b and ffe5c83.

@MylesBorins
Copy link
Contributor

@silverwind lts?

MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
PR-URL: #6132
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
Buffer:
  * Buffer.prototype.compare can now compare sub-ranges of two Buffers
   (James M Snell) #5880

deps:
  * update to http-parser 2.7.0
    (Fedor Indutny) #6279
  * update ESLint to 2.7.0
    (silverwind) #6132

net:
  * adds support for passing DNS lookup hints to createConnection()
    (Colin Ihrig) #6000

node:
  * Make the builtin libraries available for the --eval and --print
    CLI options
    (Anna Henningsen) #6207

npm:
  * upgrade npm to 3.8.6
    (Kat Marchán) #6153

repl:
  * Pressing enter in the repl will repeat the last command by default
    if no input has been received. This behaviour was in node
    previously and was not removed intentionally.
    (Rich Trott) #6090

src:
  * add SIGINFO to supported signals
    (James Reggio) #6093

streams:
  * Fix a regression that caused by net streams requesting multiple
    chunks synchronously when combined with cork/uncork
    (Matteo Collina) #6164

zlib:
  * The flushing flag is now configurable allowing for decompression
    of partial data
    (Anna Henningsen) #6069
This was referenced Apr 21, 2016
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
PR-URL: #6132
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
PR-URL: #6132
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
Buffer:
  * Buffer.prototype.compare can now compare sub-ranges of two Buffers
   (James M Snell) #5880

deps:
  * update to http-parser 2.7.0
    (Fedor Indutny) #6279
  * update ESLint to 2.7.0
    (silverwind) #6132

net:
  * adds support for passing DNS lookup hints to createConnection()
    (Colin Ihrig) #6000

node:
  * Make the builtin libraries available for the --eval and --print
    CLI options
    (Anna Henningsen) #6207

npm:
  * upgrade npm to 3.8.6
    (Kat Marchán) #6153

repl:
  * Pressing enter in the repl will repeat the last command by default
    if no input has been received. This behavior was in node
    previously and was not removed intentionally.
    (Rich Trott) #6090

src:
  * add SIGINFO to supported signals
    (James Reggio) #6093

streams:
  * Fix a regression that caused by net streams requesting multiple
    chunks synchronously when combined with cork/uncork
    (Matteo Collina) #6164

zlib:
  * The flushing flag is now configurable allowing for decompression
    of partial data
    (Anna Henningsen) #6069
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
Buffer:
  * Buffer.prototype.compare can now compare sub-ranges of two Buffers
   (James M Snell) #5880

deps:
  * update to http-parser 2.7.0
    (Fedor Indutny) #6279
  * update ESLint to 2.7.0
    (silverwind) #6132

net:
  * adds support for passing DNS lookup hints to createConnection()
    (Colin Ihrig) #6000

node:
  * Make the builtin libraries available for the --eval and --print
    CLI options
    (Anna Henningsen) #6207

npm:
  * upgrade npm to 3.8.6
    (Kat Marchán) #6153

repl:
  * Pressing enter in the repl will repeat the last command by default
    if no input has been received. This behaviour was in node
    previously and was not removed intentionally.
    (Rich Trott) #6090

src:
  * add SIGINFO to supported signals
    (James Reggio) #6093

streams:
  * Fix a regression that caused by net streams requesting multiple
    chunks synchronously when combined with cork/uncork
    (Matteo Collina) #6164

zlib:
  * The flushing flag is now configurable allowing for decompression
    of partial data
    (Anna Henningsen) #6069

PR-URL: #6322
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
Buffer:
  * Buffer.prototype.compare can now compare sub-ranges of two Buffers
   (James M Snell) #5880

deps:
  * update to http-parser 2.7.0
    (Fedor Indutny) #6279
  * update ESLint to 2.7.0
    (silverwind) #6132

net:
  * adds support for passing DNS lookup hints to createConnection()
    (Colin Ihrig) #6000

node:
  * Make the builtin libraries available for the --eval and --print
    CLI options
    (Anna Henningsen) #6207

npm:
  * upgrade npm to 3.8.6
    (Kat Marchán) #6153

repl:
  * Pressing enter in the repl will repeat the last command by default
    if no input has been received. This behaviour was in node
    previously and was not removed intentionally.
    (Rich Trott) #6090

src:
  * add SIGINFO to supported signals
    (James Reggio) #6093

streams:
  * Fix a regression that caused by net streams requesting multiple
    chunks synchronously when combined with cork/uncork
    (Matteo Collina) #6164

zlib:
  * The flushing flag is now configurable allowing for decompression
    of partial data
    (Anna Henningsen) #6069

PR-URL: #6322
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request Apr 25, 2016
Buffer:
  * Buffer.prototype.compare can now compare sub-ranges of two Buffers
   (James M Snell) nodejs#5880

deps:
  * update to http-parser 2.7.0
    (Fedor Indutny) nodejs#6279
  * update ESLint to 2.7.0
    (silverwind) nodejs#6132

net:
  * adds support for passing DNS lookup hints to createConnection()
    (Colin Ihrig) nodejs#6000

node:
  * Make the builtin libraries available for the --eval and --print
    CLI options
    (Anna Henningsen) nodejs#6207

npm:
  * upgrade npm to 3.8.6
    (Kat Marchán) nodejs#6153

repl:
  * Pressing enter in the repl will repeat the last command by default
    if no input has been received. This behaviour was in node
    previously and was not removed intentionally.
    (Rich Trott) nodejs#6090

src:
  * add SIGINFO to supported signals
    (James Reggio) nodejs#6093

streams:
  * Fix a regression that caused by net streams requesting multiple
    chunks synchronously when combined with cork/uncork
    (Matteo Collina) nodejs#6164

zlib:
  * The flushing flag is now configurable allowing for decompression
    of partial data
    (Anna Henningsen) nodejs#6069

PR-URL: nodejs#6322
jasnell pushed a commit that referenced this pull request Apr 26, 2016
PR-URL: #6132
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
jasnell pushed a commit that referenced this pull request Apr 26, 2016
PR-URL: #6132
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
jasnell pushed a commit that referenced this pull request Apr 26, 2016
Buffer:
  * Buffer.prototype.compare can now compare sub-ranges of two Buffers
   (James M Snell) #5880

deps:
  * update to http-parser 2.7.0
    (Fedor Indutny) #6279
  * update ESLint to 2.7.0
    (silverwind) #6132

net:
  * adds support for passing DNS lookup hints to createConnection()
    (Colin Ihrig) #6000

node:
  * Make the builtin libraries available for the --eval and --print
    CLI options
    (Anna Henningsen) #6207

npm:
  * upgrade npm to 3.8.6
    (Kat Marchán) #6153

repl:
  * Pressing enter in the repl will repeat the last command by default
    if no input has been received. This behaviour was in node
    previously and was not removed intentionally.
    (Rich Trott) #6090

src:
  * add SIGINFO to supported signals
    (James Reggio) #6093

streams:
  * Fix a regression that caused by net streams requesting multiple
    chunks synchronously when combined with cork/uncork
    (Matteo Collina) #6164

zlib:
  * The flushing flag is now configurable allowing for decompression
    of partial data
    (Anna Henningsen) #6069

PR-URL: #6322
MylesBorins pushed a commit that referenced this pull request May 17, 2016
PR-URL: #6132
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 17, 2016
PR-URL: #6132
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 18, 2016
PR-URL: #6132
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 18, 2016
PR-URL: #6132
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 18, 2016
Notable changes:

* deps:
  * update npm to 2.15.5 (Rebecca Turner)
    #6663
  * update ESLint to 2.7.0 (silverwind)
    #6132

* http:
  * Invalid status codes can no longer be sent. Limited to 3 digit
    numbers between 100 - 999 (Brian White)
    #6291
@MylesBorins MylesBorins mentioned this pull request May 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants