[16.x backport] tools: enable no-empty ESLint rule by Trott · Pull Request #42160 · nodejs/node

Trott · 2022-03-01T03:47:21Z

Backport of #41831

nodejs-github-bot · 2022-03-01T03:47:25Z

Review requested:

@nodejs/modules
@nodejs/startup

Trott · 2022-03-01T06:21:31Z

The lint-md failure is unrelated, but I can add a fix for it here anyway if that's desirable. It would be this:

diff --git a/doc/api/crypto.md b/doc/api/crypto.md
index 83381cffbb..255357108c 100644
--- a/doc/api/crypto.md
+++ b/doc/api/crypto.md
@@ -2532,7 +2532,7 @@ the certificate subject is considered.
 
 If the `'subject'` option is set to `'default'`, the certificate subject is only
 considered if the subject alternative name extension either does not exist or
-does not contain any DNS names. This behavior is consistent with [RFC 2818][]
+does not contain any DNS names. This behavior is consistent with \[RFC 2818]\[]
 ("HTTP Over TLS").

ljharb · 2022-03-01T06:24:40Z

@Trott it seems like that would make it be the literal text [RFC 2818][] instead of a markdown reference link defined by a [RFC 2818]: somewhere else in the document. am i misunderstanding?

Trott · 2022-03-01T06:29:27Z

@Trott it seems like that would make it be the literal text [RFC 2818][] instead of a markdown reference link defined by a [RFC 2818]: somewhere else in the document. am i misunderstanding?

Hmmm, I did a make format-md and copy/pasted the diff but now that I'm looking at it, yeah, that wouldn't be right. I wonder if there is a remark-ecosysytem update that needs to happen on v16.x-staging or else some other change to our lint rules or code.

danielleadams · 2022-03-01T16:08:41Z

@Trott @ljharb let me look into this and update the staging branch with the rest of the LTS ready commits. Hopefully, there is a fix since this didn't need to change in v17.x.

ljharb · 2022-03-01T16:10:15Z

It might just be missing the link reference itself; without that present the linter might be tripping up.

danielleadams · 2022-03-01T16:12:29Z

@ljharb Actually - you're right. I think a commit landed that changed the docs that shouldn't have. The base branch will need to be updated. Sorry for the extra work @Trott, this'll need a rebase after.

danielleadams · 2022-03-02T03:23:20Z

@Trott The base branch is ready to be rebased, thanks again!

PR-URL: nodejs#41831 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Trott · 2022-03-02T05:23:58Z

@Trott The base branch is ready to be rebased, thanks again!

OK, rebased.

Trott · 2022-03-02T05:28:41Z

(Fixing the lint errors...)

PR-URL: nodejs#41831 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Refs: https://eslint.org/docs/rules/no-empty PR-URL: nodejs#41831 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Trott · 2022-03-02T05:34:11Z

OK, now it should be ready.

PR-URL: nodejs#41831 Backport-PR-URL: nodejs#42160 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Refs: https://eslint.org/docs/rules/no-empty PR-URL: nodejs#41831 Backport-PR-URL: nodejs#42160 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: nodejs#41831 Backport-PR-URL: nodejs#42160 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Refs: https://eslint.org/docs/rules/no-empty PR-URL: nodejs#41831 Backport-PR-URL: nodejs#42160 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: #41831 Backport-PR-URL: #42160 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Refs: https://eslint.org/docs/rules/no-empty PR-URL: #41831 Backport-PR-URL: #42160 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: #41831 Backport-PR-URL: #42160 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Refs: https://eslint.org/docs/rules/no-empty PR-URL: #41831 Backport-PR-URL: #42160 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Trott requested a review from danielleadams March 1, 2022 03:47

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v16.x labels Mar 1, 2022

Trott mentioned this pull request Mar 1, 2022

tools: enable no-empty ESLint rule #41831

Closed

GeoffreyBooth approved these changes Mar 1, 2022

View reviewed changes

RaisinTen approved these changes Mar 1, 2022

View reviewed changes

danielleadams force-pushed the v16.x-staging branch from cf77590 to 1c9758b Compare March 2, 2022 03:21

Trott added 6 commits March 2, 2022 05:21

benchmark: enable no-empty ESLint rule

bf3bb92

PR-URL: nodejs#41831 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

doc: remove empty block from console.timeEnd() example

a24b61a

PR-URL: nodejs#41831 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

doc: add comments to empty blocks in worker_threads text

7c0cd14

PR-URL: nodejs#41831 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

lib: add comments to empty catch statements

0038697

PR-URL: nodejs#41831 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

policy: revise manifest.js to avoid empty blocks

b35692f

PR-URL: nodejs#41831 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

stream: remove empty block

d00c97a

PR-URL: nodejs#41831 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Trott force-pushed the v16.x-staging branch from a7f7d55 to cf022a9 Compare March 2, 2022 05:23

Trott added 2 commits March 2, 2022 05:33

test: enable no-empty ESLint rule

48f5a12

PR-URL: nodejs#41831 Refs: https://eslint.org/docs/rules/no-empty Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

tools: enable no-empty ESLint rule

01c3818

Refs: https://eslint.org/docs/rules/no-empty PR-URL: nodejs#41831 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Trott force-pushed the v16.x-staging branch from cf022a9 to 01c3818 Compare March 2, 2022 05:34

danielleadams approved these changes Mar 2, 2022

View reviewed changes

This was referenced Mar 10, 2022

CI Reliability 2022-03-10 nodejs/reliability#216

Open

CI Reliability 2022-03-11 nodejs/reliability#217

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[16.x backport] tools: enable no-empty ESLint rule#42160

[16.x backport] tools: enable no-empty ESLint rule#42160
Trott wants to merge 8 commits intonodejs:v16.x-stagingfrom
Trott:v16.x-staging

Trott commented Mar 1, 2022

Uh oh!

nodejs-github-bot commented Mar 1, 2022

Uh oh!

Trott commented Mar 1, 2022

Uh oh!

ljharb commented Mar 1, 2022

Uh oh!

Trott commented Mar 1, 2022

Uh oh!

danielleadams commented Mar 1, 2022

Uh oh!

ljharb commented Mar 1, 2022

Uh oh!

danielleadams commented Mar 1, 2022

Uh oh!

danielleadams commented Mar 2, 2022

Uh oh!

Trott commented Mar 2, 2022

Uh oh!

Trott commented Mar 2, 2022

Uh oh!

Trott commented Mar 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

Trott commented Mar 1, 2022

Uh oh!

nodejs-github-bot commented Mar 1, 2022

Uh oh!

Trott commented Mar 1, 2022

Uh oh!

ljharb commented Mar 1, 2022

Uh oh!

Trott commented Mar 1, 2022

Uh oh!

danielleadams commented Mar 1, 2022

Uh oh!

ljharb commented Mar 1, 2022

Uh oh!

danielleadams commented Mar 1, 2022

Uh oh!

danielleadams commented Mar 2, 2022

Uh oh!

Trott commented Mar 2, 2022

Uh oh!

Trott commented Mar 2, 2022

Uh oh!

Trott commented Mar 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants