tools: update ESLint to version 2.1.0 by Trott · Pull Request #5214 · nodejs/node

Trott · 2016-02-13T06:19:36Z

Update ESLint to 2.1.0. ESLint has a number of potentially-useful new
features but this change attempts to be minimal in its changes.
However, some things could not be avoided reasonably.

ESLint 2.1.0 found a few lint issues that ESLint 1.x missed, particularly
around template strings that did not take advantage of any features of
template strings, and let declarations where const sufficed.

Additionally, ESLint 2.1.0 removes some granularity around enabling ES6
features. Some features (e.g., spread operator) that had been turned off
in our configuration for ESLint 1.x are now permitted.

Trott · 2016-02-13T06:33:16Z

I hate to re-open the whole "OH NOES SHOULD WE ENABLE ES6-EVERYTHING OR NOT?!?!" question. But maybe it's easier to come to consensus if it's a "Eh, we've gotta upgrade to ESLint 2.0 sooner or later, and sooner is probably better, so..."

To hopefully clarify a bit: ESLint 2 appears to remove options that allow enabling picking and choosing your ES6 features. You can't say "Arrow functions are OK, but NO SPREAD OPERATORS!!!!" Well, you can, but it probably involves writing a custom rule. (The exception is ES6 modules which are turned off unless you take an extra step to turn them on.) This was something that ~~(I think) @targos~~@cjihrig warned was coming in some of the previous threads about whether we should continue to granularly manage ES6 features in the lint config (and, if so, whether it should be deny-all-with-exceptions or allow-all-with-disallowed-list).

On the upside, ESLint 2 seems to have found some stuff missed by ESLint 1. And it has some features we might want. Interested parties can review new rules available at http://eslint.org/blog/2016/02/eslint-v2.0.0-released#new-rules.

silverwind · 2016-02-13T12:10:02Z

~~Don't we need to update the removed rules?~~

silverwind · 2016-02-13T12:17:51Z

Nevermind, I see you made that change in the seond commit. I'd suggest moving config changes to its own commit.

cjihrig · 2016-02-13T14:54:51Z

ESLint 2 appears to remove options that allow enabling picking and choosing your ES6 features.

@rvagg this is what I was talking about in #5052.

Trott · 2016-02-13T22:08:29Z

@silverwind I put the .eslintrc changes into their own commits (6e3f278 and af3a3c6). It had to be two commits rather than one. The first commit removes rules that will break when ESLint 2 is running. That commit happens before the commit that updates ESLint 1 to ESLint 2. After the update, another commit reinstates the old behavior using the new rules. Doing all the changes to .eslintrc in its own commit (without any simultaneous changes to tools/eslint) would mean that there would be a commit somewhere that would be a broken build.

Trott · 2016-02-13T22:17:32Z

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

silverwind · 2016-02-14T04:33:10Z

Thanks! I'm fine with two commits there.

I think we need ecmaVersion: 6 in parserOptions. As I understand it, env: es6 only gives us the globals.

Trott · 2016-02-14T04:42:12Z

@silverwind According to the docs, specifying es6 as an env enables all ES6 features except modules. So I don't think we need to use parserOptions too (although I suppose it wouldn't cause problems either). The lint results seem to bear that out.

silverwind · 2016-02-14T04:56:05Z

Ah, your're right, the parser option is set by the env:

https://github.com/eslint/eslint/blob/5aa474833b161b2d5eb684b911d1f7745fa2a696/conf/environments.js#L99-L103

silverwind · 2016-02-14T04:56:57Z

LGTM

targos · 2016-02-15T07:21:19Z

When I updated to v1, I used dmn to reduce the number of files. Could you do the same ?
FWIW here is the full procedure I followed: https://gist.github.com/targos/f2de3a723a4ec54290ee

targos · 2016-02-15T07:29:30Z

Can you please make sure that the tools: update ESLint to version 2.0 commit is only about the changes tools/eslint ? I see some in tools/doc and tools/eslint-rules.

Trott · 2016-02-15T15:36:58Z

@targos wrote:

Can you please make sure that the tools: update ESLint to version 2.0 commit is only about the changes tools/eslint ? I see some in tools/doc and tools/eslint-rules.

Ugh, yes, that was a mistake. I'll fix that.

Trott · 2016-02-15T15:38:24Z

@targos wrote:

When I updated to v1, I used dmn to reduce the number of files. Could you do the same ?

Oh, yes, definitely! I did not know about dmn.

jasnell · 2016-02-15T15:56:07Z

LGTM

Trott · 2016-02-15T16:12:59Z

@targos The reworking of the commits and using dmn are all done now. Thanks! (The gist was really helpful, unsurprisingly.)

Trott · 2016-02-15T16:14:08Z

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

MylesBorins · 2016-02-15T19:06:26Z

Looks like eslint has updated to v2.1.0 already. Might want to bump it in this PR

Trott · 2016-02-18T00:21:13Z

Rebased against master. Another CI: https://ci.nodejs.org/job/node-test-pull-request/1693/

Trott · 2016-02-18T05:14:06Z

Current status:

CI is green
3 LGTMs
Lands cleanly on v4.x-staging except for three conflicts which are all easy to resolve.

OK to land this on master now, @thealphanerd, or do you still have v4.x concerns? I'm happy to hold off if there are concerns, but I'd also really like to move forward if there aren't any.

MylesBorins · 2016-02-18T07:39:44Z

Lgtm!

silverwind · 2016-02-18T08:17:16Z

jbergstroem · 2016-02-18T14:04:29Z

LGTM (I knew you were waiting for me! 😣)

ESLint 2.0 is coming. Some lint rules have been tightened. PR-URL: nodejs#5214

We are about to upgrade from ESlint 1 to ESLint 2. Remove lint rules that will not exist in ESLint 2.

Update ESLint to 2.1.0. ESLint has a number of potentially-useful new features but this change attempts to be minimal in its changes. However, some things could not be avoided reasonably. ESLint 2.1.0 found a few lint issues that ESLing 1.x missed with template strings that did not take advantage of any features of template strings, and `let` declarations where `const` sufficed. Additionally, ESLint 2.1.0 removes some granularity around enabling ES6 features. Some features (e.g., spread operator) that had been turned off in our configuration for ESLint 1.x are now permitted. PR-URL: nodejs#5214

Now that we are using ESLint 2, replace ESLint 1 rules with their ESLint 2 equivalents. PR-URL: nodejs#5214

Trott · 2016-02-18T18:00:40Z

OK, one last rebase against master and a CI run before landing.

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

ESLint 2.1.0 is coming. Some lint rules have been tightened. PR-URL: nodejs#5214 Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: jbergstroem - Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Myles Borins <myles.borins@gmail.com>

We are about to upgrade from ESlint 1 to ESLint 2. Remove lint rules that will not exist in ESLint 2. PR-URL: nodejs#5214 Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: jbergstroem - Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Myles Borins <myles.borins@gmail.com>

Update ESLint to 2.1.0. ESLint has a number of potentially-useful new features but this change attempts to be minimal in its changes. However, some things could not be avoided reasonably. ESLint 2.1.0 found a few lint issues that ESLing 1.x missed with template strings that did not take advantage of any features of template strings, and `let` declarations where `const` sufficed. Additionally, ESLint 2.1.0 removes some granularity around enabling ES6 features. Some features (e.g., spread operator) that had been turned off in our configuration for ESLint 1.x are now permitted. PR-URL: nodejs#5214 Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: jbergstroem - Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Myles Borins <myles.borins@gmail.com>

Now that we are using ESLint 2, replace ESLint 1 rules with their ESLint 2 equivalents. PR-URL: nodejs#5214 Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: jbergstroem - Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Myles Borins <myles.borins@gmail.com>

Trott · 2016-02-18T18:41:56Z

Landed in 32f6098, d3c3a92, d7aa8fa, and 2ba7baa.

ESLint 2.1.0 is coming. Some lint rules have been tightened. PR-URL: #5214 Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: jbergstroem - Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Myles Borins <myles.borins@gmail.com>

We are about to upgrade from ESlint 1 to ESLint 2. Remove lint rules that will not exist in ESLint 2. PR-URL: #5214 Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: jbergstroem - Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Myles Borins <myles.borins@gmail.com>

Update ESLint to 2.1.0. ESLint has a number of potentially-useful new features but this change attempts to be minimal in its changes. However, some things could not be avoided reasonably. ESLint 2.1.0 found a few lint issues that ESLing 1.x missed with template strings that did not take advantage of any features of template strings, and `let` declarations where `const` sufficed. Additionally, ESLint 2.1.0 removes some granularity around enabling ES6 features. Some features (e.g., spread operator) that had been turned off in our configuration for ESLint 1.x are now permitted. PR-URL: #5214 Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: jbergstroem - Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Myles Borins <myles.borins@gmail.com>

Now that we are using ESLint 2, replace ESLint 1 rules with their ESLint 2 equivalents. PR-URL: #5214 Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: jbergstroem - Johan Bergström <bugs@bergstroem.nu> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Myles Borins <myles.borins@gmail.com>

MylesBorins · 2016-02-22T20:58:34Z

@Trott I think this one may need to be manually backported... would you be up for it?

Trott · 2016-02-22T23:25:36Z

@thealphanerd Here you go: #5372

Trott added tools Issues and PRs related to the tools directory. lts-watch-v4.x labels Feb 13, 2016

Trott force-pushed the eslint2 branch from 6bc45f6 to af3a3c6Compare February 13, 2016 22:03

Trott force-pushed the eslint2 branch from af3a3c6 to 5fe022bCompare February 15, 2016 16:12

Trott force-pushed the eslint2 branch from 5fe022b to 3f886d2Compare February 15, 2016 22:42

Trott added 4 commits February 18, 2016 09:58

src,test,tools: modify for more stringent linting
0d37bb9
ESLint 2.0 is coming. Some lint rules have been tightened. PR-URL: nodejs#5214

tools: remove obsolete lint rules
1794253
We are about to upgrade from ESlint 1 to ESLint 2. Remove lint rules that will not exist in ESLint 2.

tools: replace obsolete ESLint rules
7dd31f5
Now that we are using ESLint 2, replace ESLint 1 rules with their ESLint 2 equivalents. PR-URL: nodejs#5214

Trott force-pushed the eslint2 branch from 97d41e4 to 7dd31f5Compare February 18, 2016 17:59

Trott closed this Feb 18, 2016

Trott mentioned this pull request Feb 22, 2016
V4 eslint 2 #5372
Closed

MylesBorins mentioned this pull request Mar 1, 2016
V4.4.0 proposal #5301
Merged

Uh oh!

Uh oh!

Conversation

Trott commented Feb 13, 2016

Uh oh!

Trott commented Feb 13, 2016

Uh oh!

silverwind commented Feb 13, 2016

Uh oh!

silverwind commented Feb 13, 2016

Uh oh!

cjihrig commented Feb 13, 2016

Uh oh!

Trott commented Feb 13, 2016

Uh oh!

Trott commented Feb 13, 2016

Uh oh!

silverwind commented Feb 14, 2016

Uh oh!

Trott commented Feb 14, 2016

Uh oh!

silverwind commented Feb 14, 2016

Uh oh!

silverwind commented Feb 14, 2016

Uh oh!

targos commented Feb 15, 2016

Uh oh!

targos commented Feb 15, 2016

Uh oh!

Trott commented Feb 15, 2016

Uh oh!

Trott commented Feb 15, 2016

Uh oh!

jasnell commented Feb 15, 2016

Uh oh!

Trott commented Feb 15, 2016

Uh oh!

Trott commented Feb 15, 2016

Uh oh!

MylesBorins commented Feb 15, 2016

Uh oh!

Trott commented Feb 18, 2016

Uh oh!

Trott commented Feb 18, 2016

Uh oh!

MylesBorins commented Feb 18, 2016 via email

Uh oh!

silverwind commented Feb 18, 2016

Uh oh!

jbergstroem commented Feb 18, 2016

Uh oh!

Trott commented Feb 18, 2016

Uh oh!

Trott commented Feb 18, 2016

Uh oh!

MylesBorins commented Feb 22, 2016

Uh oh!

Trott commented Feb 22, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants