tools: apply stricter indentation rules to tools by Trott · Pull Request #13758 · nodejs/node · GitHub

ESLint 4.0.0 provides stricter (and more granular) indentation checking
than previous versions. Apply the stricter indentation rules to the
tools directory.

In preparation for applying the more strict indentation linting
available in ESLint 4.0.0, correct minor indentation issues in
tools/eslint-rules/required-modules.js.

This is the only file with indentation that does not conform to the
stricter checks.

Checklist

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

Affected core subsystem(s)

tools

Unrelated to this PR: Should this line be return foundModules.indexOf(module) === -1; instead?

or even requiredModules.filter(module => !foundModules.includes(module))

Ack, fixed.

Oh cool, I wonder how this applies to test/...

./node tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules --ext=.js test
✖ 2460 problems (2460 errors, 0 warnings)
  2460 errors, 0 warnings potentially fixable with the `--fix` option.

Okay, wonder how many can be --fix ed

./node tools/eslint/bin/eslint.js --cache --fix --rulesdir=tools/eslint-rules --ext=.js test
✖ 1923 problems (1923 errors, 0 warnings)
  1917 errors, 0 warnings potentially fixable with the `--fix` option.

@not-an-aardvark do you know why things like this can't be --fixed? Is it just a question of someone implementing it, or is is more complex (i.e. the surrounding lines would have to be refactored)?

/Users/gib/wrk/com/node/test/sequential/test-pipe.js
  104:1  error  Expected indentation of 6 spaces but found 4  indent
  105:1  error  Expected indentation of 6 spaces but found 4  indent
  106:9  error  Expected indentation of 6 spaces but found 8  indent-legacy
  107:9  error  Expected indentation of 6 spaces but found 8  indent-legacy
  108:7  error  Expected indentation of 4 spaces but found 6  indent-legacy
  109:1  error  Expected indentation of 4 spaces but found 2  indent

I guess you need to disable indent-legacy.

Looking at the docs, I thought indent was a superset of indent-legacy, so I assumed that wouldn't be an issue, but I could be wrong.

It is mostly a superset, but I think there are a few cases where indent-legacy has a bug and tries to enforce the wrong indentation.

In preparation for applying the more strict indentation linting available in ESLint 4.0.0, correct minor indentation issues in tools/eslint-rules/required-modules.js. This is the only file with indentation that does not conform to the stricter checks.

ESLint 4.0.0 provides stricter (and more granular) indentation checking than previous versions. Apply the stricter indentation rules to the tools directory.

Fix previously-unnoticed typo in `required-modules.js`. Refs: nodejs#13758 (comment)

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

LGTM

In preparation for applying the more strict indentation linting available in ESLint 4.0.0, correct minor indentation issues in tools/eslint-rules/required-modules.js. This is the only file with indentation that does not conform to the stricter checks. PR-URL: nodejs#13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

ESLint 4.0.0 provides stricter (and more granular) indentation checking than previous versions. Apply the stricter indentation rules to the tools directory. PR-URL: nodejs#13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Fix previously-unnoticed typo in `required-modules.js`. Refs: nodejs#13758 (comment) PR-URL: nodejs#13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Landed in 491f838, ea36609, and 25b4d87

In preparation for applying the more strict indentation linting available in ESLint 4.0.0, correct minor indentation issues in tools/eslint-rules/required-modules.js. This is the only file with indentation that does not conform to the stricter checks. PR-URL: #13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

ESLint 4.0.0 provides stricter (and more granular) indentation checking than previous versions. Apply the stricter indentation rules to the tools directory. PR-URL: #13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Fix previously-unnoticed typo in `required-modules.js`. Refs: #13758 (comment) PR-URL: #13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

This does not land cleanly in LTS. Please feel free to manually backport. Please also feel free to replace the backport request label with do-not-land if it shouldn't land

@MylesBorins Some required stuff probably landed in the last 12 hours, as all three commits here cherry-pick cleanly for me to v6.x-staging now. Can you try again?

@Trott getting failures when landing this

/usr/local/opt/python/bin/python2.7 tools/test.py --mode=release -J \
		doctool inspector known_issues message pseudo-tty parallel sequential addons
[02:04|% 100|+ 1357|-   0]: Done
/Applications/Xcode.app/Contents/Developer/usr/bin/make lint
Running JS linter...
./node tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules \
	  benchmark lib test tools
/Users/mborins/code/node/v6.x/tools/.eslintrc.yaml:
	Configuration for rule "indent" is invalid:
	Value "off" is the wrong type.

Error: /Users/mborins/code/node/v6.x/tools/.eslintrc.yaml:
	Configuration for rule "indent" is invalid:
	Value "off" is the wrong type.

    at validateRuleOptions (/Users/mborins/code/node/v6.x/tools/eslint/lib/config/config-validator.js:109:15)
    at Object.keys.forEach.id (/Users/mborins/code/node/v6.x/tools/eslint/lib/config/config-validator.js:156:13)
    at Array.forEach (native)
    at Object.validate (/Users/mborins/code/node/v6.x/tools/eslint/lib/config/config-validator.js:155:35)
    at Object.load (/Users/mborins/code/node/v6.x/tools/eslint/lib/config/config-file.js:559:19)
    at loadConfig (/Users/mborins/code/node/v6.x/tools/eslint/lib/config.js:63:33)
    at getLocalConfig (/Users/mborins/code/node/v6.x/tools/eslint/lib/config.js:130:29)
    at Config.getConfig (/Users/mborins/code/node/v6.x/tools/eslint/lib/config.js:260:26)
    at hashOfConfigFor (/Users/mborins/code/node/v6.x/tools/eslint/lib/cli-engine.js:599:41)
    at executeOnFile (/Users/mborins/code/node/v6.x/tools/eslint/lib/cli-engine.js:648:32)

In preparation for applying the more strict indentation linting available in ESLint 4.0.0, correct minor indentation issues in tools/eslint-rules/required-modules.js. This is the only file with indentation that does not conform to the stricter checks. PR-URL: nodejs#13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

ESLint 4.0.0 provides stricter (and more granular) indentation checking than previous versions. Apply the stricter indentation rules to the tools directory. PR-URL: nodejs#13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Fix previously-unnoticed typo in `required-modules.js`. Refs: nodejs#13758 (comment) PR-URL: nodejs#13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

@MylesBorins Backport in #14360

In preparation for applying the more strict indentation linting available in ESLint 4.0.0, correct minor indentation issues in tools/eslint-rules/required-modules.js. This is the only file with indentation that does not conform to the stricter checks. Backport-PR-URL: #14360 PR-URL: #13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

ESLint 4.0.0 provides stricter (and more granular) indentation checking than previous versions. Apply the stricter indentation rules to the tools directory. Backport-PR-URL: #14360 PR-URL: #13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Fix previously-unnoticed typo in `required-modules.js`. Backport-PR-URL: #14360 Refs: #13758 (comment) PR-URL: #13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

In preparation for applying the more strict indentation linting available in ESLint 4.0.0, correct minor indentation issues in tools/eslint-rules/required-modules.js. This is the only file with indentation that does not conform to the stricter checks. Backport-PR-URL: #14360 PR-URL: #13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

ESLint 4.0.0 provides stricter (and more granular) indentation checking than previous versions. Apply the stricter indentation rules to the tools directory. Backport-PR-URL: #14360 PR-URL: #13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Fix previously-unnoticed typo in `required-modules.js`. Backport-PR-URL: #14360 Refs: #13758 (comment) PR-URL: #13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

In preparation for applying the more strict indentation linting available in ESLint 4.0.0, correct minor indentation issues in tools/eslint-rules/required-modules.js. This is the only file with indentation that does not conform to the stricter checks. Backport-PR-URL: #14360 PR-URL: #13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

ESLint 4.0.0 provides stricter (and more granular) indentation checking than previous versions. Apply the stricter indentation rules to the tools directory. Backport-PR-URL: #14360 PR-URL: #13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Fix previously-unnoticed typo in `required-modules.js`. Backport-PR-URL: #14360 Refs: #13758 (comment) PR-URL: #13758 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Trott added the tools Issues and PRs related to the tools directory. label Jun 18, 2017

not-an-aardvark reviewed Jun 18, 2017

View reviewed changes

not-an-aardvark approved these changes Jun 18, 2017

View reviewed changes

vsemozhetbyt approved these changes Jun 18, 2017

View reviewed changes

cjihrig approved these changes Jun 18, 2017

View reviewed changes

jasnell approved these changes Jun 19, 2017

View reviewed changes

Trott added 3 commits June 19, 2017 11:11

tools: apply stricter indentation rules to tools

9a2a30c

ESLint 4.0.0 provides stricter (and more granular) indentation checking than previous versions. Apply the stricter indentation rules to the tools directory.

tools: fix error in custom ESLint rule

8b834c3

Fix previously-unnoticed typo in `required-modules.js`. Refs: nodejs#13758 (comment)

Trott force-pushed the tools-indenting branch from dd13428 to 8b834c3 Compare June 19, 2017 18:14

mhdawson approved these changes Jun 20, 2017

View reviewed changes

Trott closed this Jun 23, 2017

addaleax mentioned this pull request Jun 24, 2017

v8.1.3 proposal #13861

Merged

addaleax mentioned this pull request Jun 24, 2017

v8.2.0 proposal #13744

Merged

MylesBorins added the backport-requested-v6.x label Jul 17, 2017

Trott mentioned this pull request Jul 19, 2017

[v6.x backport] tools: apply stricter indentation rules to tools #14360

Closed

MylesBorins added backported-to-v6.x and removed backport-requested-v6.x labels Jul 21, 2017

MylesBorins mentioned this pull request Jul 21, 2017

v6.11.2 proposal #14356

Merged

Trott deleted the tools-indenting branch January 13, 2022 22:45

tools: apply stricter indentation rules to tools by Trott · Pull Request #13758 · nodejs/node · GitHub

Uh oh!

Uh oh!

tools: apply stricter indentation rules to tools - #13758

tools: apply stricter indentation rules to tools #13758Trott wants to merge 3 commits intonodejs:masternodejs/node:masterfrom Trott:tools-indentingTrott/io.js:tools-indentingCopy head branch name to clipboard

Conversation

Trott commented Jun 18, 2017

Checklist

Affected core subsystem(s)

Uh oh!

not-an-aardvark Jun 18, 2017

Choose a reason for hiding this comment

Uh oh!

TimothyGu Jun 18, 2017

Choose a reason for hiding this comment

Uh oh!

Trott Jun 19, 2017

Choose a reason for hiding this comment

Uh oh!

gibfahn commented Jun 18, 2017

Uh oh!

targos commented Jun 18, 2017

Uh oh!

gibfahn commented Jun 18, 2017

Uh oh!

not-an-aardvark commented Jun 18, 2017

Uh oh!

Trott commented Jun 19, 2017

Uh oh!

mhdawson left a comment

Choose a reason for hiding this comment

Uh oh!

Trott commented Jun 23, 2017

Uh oh!

MylesBorins commented Jul 17, 2017

Uh oh!

Trott commented Jul 18, 2017

Uh oh!

MylesBorins commented Jul 18, 2017

Uh oh!

Trott commented Jul 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

tools: apply stricter indentation rules to tools #13758
Trott wants to merge 3 commits into
nodejs:masterfrom
Trott:tools-indenting