Eslint 4.1.0 #13895

Trott · 2017-06-24T04:03:42Z

First commit:

tools: update to ESLint 4.1.0

Update ESLint to 4.1.0. This fixes a bug that previously prevented us
from using the new and stricter indentation checking.

Refs: https://github.com/eslint/eslint/issues/8721

Second commit:

tools: add script to update ESLint

Provide a bash script for updating ESLint in the project.

Third commit:

tools,benchmark: use stricter indentation linting

Enable stricter indentation rules for benchmark code.

Fourth commit:

tools: disable legacy indentation linting in tools

The tools directory has newer and stricter indentation enabled, but the
legacy indentation rules were not disabled. This could potentitally
result in a conflict between the two rule sets. Disable legacy linting.

Checklist

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

Affected core subsystem(s)

tools benchmark

richardlau · 2017-06-24T04:29:01Z

tools/update-eslint.sh

+
+# This symlink will be pointed at the wrong place after all the moving after 
+# everything is moved around so let's remove it.
+rm node_modules/.bin/eslint


Have we tried passing --no-bin-links to npm install to prevent the symlink being created in the first place?

Didn't know about that flag. Added! Thanks!

richardlau · 2017-06-24T04:45:09Z

tools/update-eslint.sh

@@ -0,0 +1,30 @@
+#!/bin/sh


Should this be bash instead of sh?

More specifically, #!/usr/bin/env bash?

Yes, that would be better, done, thanks!

richardlau · 2017-06-24T04:49:14Z

👍 for scripting the update process.

Trott · 2017-06-24T05:32:00Z

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

refack

Not sure I love the node_modules shuffle trick

refack · 2017-06-24T21:08:29Z

tools/update-eslint.sh

+# eslint-plugin-markdown is pinned at 1.0.0-beta.4 until there is a release
+# that fixes https://github.com/eslint/eslint-plugin-markdown/issues/69.
+npm install --no-bin-links --production eslint@latest eslint-plugin-markdown@1.0.0-beta.4
+rm package-lock.json 


Why? And if you really don't want it add --no-shrinkwrap to the line above.

Good point, removed it the rm, thanks.

refack · 2017-06-24T21:10:15Z

tools/update-eslint.sh

+dmn -f clean
+
+# Move stuff to where it goes in our source tree.
+mv node_modules/eslint eslint


~~A bit smelly?~~
Prefer to do in eslint-tmp.

I used --global-style to avoid having to move things in node_modules. The problem is I don't know how to install the markdown plugin this way.

refack · 2017-06-24T21:20:02Z

Why not curl https://github.com/eslint/eslint/archive/v4.1.0.tar.gz then npm i ?
To me it seems more straightforward 🤷‍♂️

Trott · 2017-06-24T21:33:36Z

Why not curl https://github.com/eslint/eslint/archive/v4.1.0.tar.gz then npm i

@refack That assumes we know which version we want to update to. If there's an archive/latest.tar.gz or something like that, then that would work, I think.

Trott · 2017-06-24T21:34:55Z

(I don't want to have to update the script every time I wanted to update, but I guess if I had to, that's not exactly the end of the world. It would also document what version the script was last run with, in case someone starts updating without the script again. So maybe it's a good thing? I dunno...)

Trott · 2017-06-24T22:19:56Z

Another thing to consider: There's also the issue that since ESLint documents the npm install method only (at least as far as I can see), it might be a good idea (in theory at least?) to avoid devising our own method like that.

refack · 2017-06-24T22:29:20Z

Another thing to consider: There's also the issue that since ESLint documents the npm install method only (at least as far as I can see), it might be a good idea (in theory at least?) to avoid devising our own method like that.

I'd assume they support npm i from git.... But I get what you're saying. Only thing I'd change then is to do the npm i eslint in a temp directory (e.g. eslint-temp) then move things out from there and rimraf it, maybe at some point we'll want to have tools/node_modules...

Trott · 2017-06-24T23:07:04Z

Good point about tools/node_modules...

Update ESLint to 4.1.0. This fixes a bug that previously prevented us from using the new and stricter indentation checking. Refs: eslint/eslint#8721

Provide a bash script for updating ESLint in the project.

Enable stricter indentation rules for benchmark code.

The tools directory has newer and stricter indentation enabled, but the legacy indentation rules were not disabled. This could potentitally result in a conflict between the two rule sets. Disable legacy linting.

Trott · 2017-06-27T03:04:43Z

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

targos · 2017-06-27T04:56:52Z

LGTM

Trott · 2017-06-27T13:25:43Z

Landed in aaee434 1e5e0ce 496f604
and b714060

MylesBorins · 2017-08-14T20:31:59Z

@Trott backport?

Trott · 2017-08-14T21:00:17Z

Backport the first two but not the last two would be my suggestion. Coming in a few minutes...

Update ESLint to 4.1.0. This fixes a bug that previously prevented us from using the new and stricter indentation checking. Refs: eslint/eslint#8721 PR-URL: nodejs#13895 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Provide a bash script for updating ESLint in the project. PR-URL: nodejs#13895 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Trott · 2017-08-14T21:06:32Z

@MylesBorins #14830

Update ESLint to 4.1.0. This fixes a bug that previously prevented us from using the new and stricter indentation checking. Refs: eslint/eslint#8721 PR-URL: nodejs#13895 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Provide a bash script for updating ESLint in the project. PR-URL: nodejs#13895 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Update ESLint to 4.1.0. This fixes a bug that previously prevented us from using the new and stricter indentation checking. Refs: eslint/eslint#8721 PR-URL: #13895 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Provide a bash script for updating ESLint in the project. PR-URL: #13895 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Update ESLint to 4.1.0. This fixes a bug that previously prevented us from using the new and stricter indentation checking. Refs: eslint/eslint#8721 Backport-PR-URL: #14830 PR-URL: #13895 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Provide a bash script for updating ESLint in the project. Backport-PR-URL: #14830 PR-URL: #13895 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Update ESLint to 4.1.0. This fixes a bug that previously prevented us from using the new and stricter indentation checking. Refs: eslint/eslint#8721 Backport-PR-URL: #14830 PR-URL: #13895 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Provide a bash script for updating ESLint in the project. Backport-PR-URL: #14830 PR-URL: #13895 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Trott added benchmark Issues and PRs related to the benchmark subsystem. tools Issues and PRs related to the tools directory. labels Jun 24, 2017

richardlau reviewed Jun 24, 2017

View reviewed changes

Trott force-pushed the eslint-4.1.0 branch 2 times, most recently from ac3fbd7 to adb3ef9 Compare June 24, 2017 05:26

cjihrig approved these changes Jun 24, 2017

View reviewed changes

refack reviewed Jun 24, 2017

View reviewed changes

Trott force-pushed the eslint-4.1.0 branch from adb3ef9 to 85236cb Compare June 24, 2017 21:28

silverwind approved these changes Jun 25, 2017

View reviewed changes

Trott force-pushed the eslint-4.1.0 branch from 85236cb to 193d871 Compare June 27, 2017 02:35

Trott added 4 commits June 26, 2017 19:58

tools: update to ESLint 4.1.0

8e358a0

Update ESLint to 4.1.0. This fixes a bug that previously prevented us from using the new and stricter indentation checking. Refs: eslint/eslint#8721

tools: add script to update ESLint

2981120

Provide a bash script for updating ESLint in the project.

tools,benchmark: use stricter indentation linting

3d89abe

Enable stricter indentation rules for benchmark code.

tools: disable legacy indentation linting in tools

923d6d6

The tools directory has newer and stricter indentation enabled, but the legacy indentation rules were not disabled. This could potentitally result in a conflict between the two rule sets. Disable legacy linting.

Trott force-pushed the eslint-4.1.0 branch from 193d871 to 923d6d6 Compare June 27, 2017 03:03

jasnell approved these changes Jun 27, 2017

View reviewed changes

refack approved these changes Jun 27, 2017

View reviewed changes

Trott closed this Jun 27, 2017

MylesBorins added the backport-requested-v6.x label Aug 14, 2017

Trott mentioned this pull request Aug 14, 2017

[v6.x backport] Eslint 4.1.0 #14830

Closed

4 tasks

MylesBorins added backported-to-v6.x and removed backport-requested-v6.x labels Aug 16, 2017

MylesBorins mentioned this pull request Aug 16, 2017

v6.11.3-proposal #14852

Merged

refack mentioned this pull request Aug 17, 2017

tools: remove stray package-lock.json file #14873

Closed

1 task

lpinca mentioned this pull request Mar 1, 2021

tools: update-eslint.sh does not work with npm@7 #37560

Closed

Trott deleted the eslint-4.1.0 branch January 13, 2022 22:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eslint 4.1.0 #13895

Eslint 4.1.0 #13895

Trott commented Jun 24, 2017

richardlau Jun 24, 2017

Trott Jun 24, 2017

richardlau Jun 24, 2017

richardlau Jun 24, 2017

Trott Jun 24, 2017

richardlau commented Jun 24, 2017

Trott commented Jun 24, 2017

refack left a comment

refack Jun 24, 2017

Trott Jun 24, 2017

refack Jun 24, 2017 •

edited

Loading

targos Jun 25, 2017

refack commented Jun 24, 2017

Trott commented Jun 24, 2017

Trott commented Jun 24, 2017

Trott commented Jun 24, 2017

refack commented Jun 24, 2017

Trott commented Jun 24, 2017

Trott commented Jun 27, 2017

targos commented Jun 27, 2017

Trott commented Jun 27, 2017

MylesBorins commented Aug 14, 2017

Trott commented Aug 14, 2017

Trott commented Aug 14, 2017

Eslint 4.1.0 #13895

Eslint 4.1.0 #13895

Conversation

Trott commented Jun 24, 2017

Checklist

Affected core subsystem(s)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richardlau commented Jun 24, 2017

Trott commented Jun 24, 2017

refack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

refack Jun 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

refack commented Jun 24, 2017

Trott commented Jun 24, 2017

Trott commented Jun 24, 2017

Trott commented Jun 24, 2017

refack commented Jun 24, 2017

Trott commented Jun 24, 2017

Trott commented Jun 27, 2017

targos commented Jun 27, 2017

Trott commented Jun 27, 2017

MylesBorins commented Aug 14, 2017

Trott commented Aug 14, 2017

Trott commented Aug 14, 2017

refack Jun 24, 2017 •

edited

Loading