tools: enable ESLint recommended rule set

[Trott]

No description provided.

[nodejs-github-bot]

Review requested:

• @nodejs/modules

[bnb]

It seems there's a mix of eslint-disable-next-line and eslint-disable-line. Would we want to standardize on one and roll with that?

[ljharb]

Is there a reason not to permit both? I usually use whichever one is more readable, depending on the line length of the line i'm trying to disable the warning for.

[Trott]

It seems there's a mix of eslint-disable-next-line and eslint-disable-line. Would we want to standardize on one and roll with that?

I wouldn't want to standardize on one, but if we did, it would have to be eslint-disable-next-line because eslint-disable-line will sometimes result in a max-len violation.

[ljharb]

(max-len should be disabled anyways; line length limits are a terrible way to manage complexity; but that’s a discussion not worth having here :-) )

[Trott]

(max-len should be disabled anyways; line length limits are a terrible way to manage complexity; but that’s a discussion not worth having here :-) )

I agree and have advocated for this in the past but met significant resistance. It might be time to bring it up again.

[bnb]

I agree and have advocated for this in the past but met significant resistance. It might be time to bring it up again.

I also agree and would be massively supportive.

[bnb]

I wouldn't want to standardize on one

no worries then. I figured it might be more readable (it is for my brain, since it requires less processing to match) but if we don't want to that's totally reasonable.

[nodejs-github-bot]

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

[GeoffreyBooth]

I agree and have advocated for this in the past but met significant resistance. It might be time to bring it up again.

I also agree and would be massively supportive.

+100. This is one of the biggest pains in getting code working, and it actually makes debugging harder. The current length is so short that lots of code is less readable because things like function calls and if conditionals need to be broken onto several lines.

[nodejs-github-bot]

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

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/41463
✔ Done loading data for nodejs/node/pull/41463
----------------------------------- PR info ------------------------------------
Title tools: enable ESLint recommended rule set (#41463)
Author Rich Trott (@Trott)
Branch Trott:eslint-recommended-3 -> nodejs:master
Labels util, tools, esm, needs-ci, commit-queue-rebase
Commits 6
- tools: enable ESLint no-loss-of-precision rule
- tools: enable ESLint no-sparse-arrays rule
- tools: enable ESLint require-yield rule
- tools,lib.test: enable ESLint no-regex-spaces rule
- tools: enable ESLint no-constant-condition rule
- tools: enable ESLint recommended configuration
Committers 1
- Rich Trott PR-URL: https://github.com/nodejs/node/pull/41463
Reviewed-By: Tobias Nießen Reviewed-By: Michaël Zasso Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig Reviewed-By: Geoffrey Booth ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/41463
Reviewed-By: Tobias Nießen Reviewed-By: Michaël Zasso Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig Reviewed-By: Geoffrey Booth --------------------------------------------------------------------------------
ℹ This PR was created on Mon, 10 Jan 2022 19:59:29 GMT
✔ Approvals: 5
✔ - Tobias Nießen (@tniessen) (TSC): https://github.com/nodejs/node/pull/41463#pullrequestreview-848282703
✔ - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/41463#pullrequestreview-848292420
✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/41463#pullrequestreview-848298916
✔ - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/41463#pullrequestreview-848305961
✔ - Geoffrey Booth (@GeoffreyBooth): https://github.com/nodejs/node/pull/41463#pullrequestreview-852251982
✖ GitHub CI is still running
ℹ Last Full PR CI on 2022-01-13T22:20:05Z: https://ci.nodejs.org/job/node-test-pull-request/41860/
- Querying data for job/node-test-pull-request/41860/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/1695349357