Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tools: move ESLint to tools/eslint #53413

Merged
merged 5 commits into from
Jun 19, 2024
Merged

tools: move ESLint to tools/eslint #53413

merged 5 commits into from
Jun 19, 2024

Conversation

targos
Copy link
Member

@targos targos commented Jun 11, 2024

Greatly simplify how ESLint and its plugins are installed.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions
  • @nodejs/startup
  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Jun 11, 2024
@targos
Copy link
Member Author

targos commented Jun 11, 2024

Note that inikulin/dmn#56 is necessary to make the update script work correctly. I don't know if we should block this PR on it.

@targos
tools: move ESLint to tools/eslint
34f3770 
Greatly simplify how ESLint and its plugins are installed.
@targos
Copy link
Member Author

targos commented Jun 11, 2024

I split the commit in two so the non-automated changes can be reviewed.

@targos targos added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Jun 11, 2024
@targos
Copy link
Member Author

targos commented Jun 11, 2024

I don't know why it adds >2000 new lines and what would be a good way to know it.

@aduh95
Copy link
Contributor

aduh95 commented Jun 11, 2024

I don't know why it adds >2000 new lines and what would be a good way to know it.

git show f917d220363ff0fbb479e18b7ef9176e76d373de --diff-filter=A maybe?

@targos

This comment was marked as off-topic.

Sign in to view
@targos
Copy link
Member Author

targos commented Jun 11, 2024

Forget about my previous comment. I did the analysis on another commit.

@targos
Copy link
Member Author

targos commented Jun 11, 2024

I downgraded browserslist and caniuse-lite so they're at the same version as the main branch to reduce the changes.

@targos
Copy link
Member Author

targos commented Jun 11, 2024

Now we don't have too many added files:

$ git show eb0f9bcde1cad9fb76d2509162ce06c89bb54a93 --diff-filter=A | grep "+++ b"
+++ b/tools/eslint/node_modules/eslint/node_modules/escape-string-regexp/index.js
+++ b/tools/eslint/node_modules/eslint/node_modules/escape-string-regexp/package.json
+++ b/tools/eslint/node_modules/eslint/node_modules/escape-string-regexp/readme.md
+++ b/tools/eslint/node_modules/eslint/node_modules/eslint-visitor-keys/LICENSE
+++ b/tools/eslint/node_modules/eslint/node_modules/eslint-visitor-keys/dist/eslint-visitor-keys.cjs
+++ b/tools/eslint/node_modules/eslint/node_modules/eslint-visitor-keys/dist/eslint-visitor-keys.d.cts
+++ b/tools/eslint/node_modules/eslint/node_modules/eslint-visitor-keys/lib/index.js
+++ b/tools/eslint/node_modules/eslint/node_modules/eslint-visitor-keys/lib/visitor-keys.js
+++ b/tools/eslint/node_modules/eslint/node_modules/eslint-visitor-keys/package.json
+++ b/tools/eslint/node_modules/espree/node_modules/eslint-visitor-keys/LICENSE
+++ b/tools/eslint/node_modules/espree/node_modules/eslint-visitor-keys/dist/eslint-visitor-keys.cjs
+++ b/tools/eslint/node_modules/espree/node_modules/eslint-visitor-keys/dist/eslint-visitor-keys.d.cts
+++ b/tools/eslint/node_modules/espree/node_modules/eslint-visitor-keys/lib/index.js
+++ b/tools/eslint/node_modules/espree/node_modules/eslint-visitor-keys/lib/visitor-keys.js
+++ b/tools/eslint/node_modules/espree/node_modules/eslint-visitor-keys/package.json
+++ b/tools/eslint/node_modules/esquery/node_modules/estraverse/estraverse.js
+++ b/tools/eslint/node_modules/esquery/node_modules/estraverse/package.json
+++ b/tools/eslint/node_modules/esrecurse/node_modules/estraverse/estraverse.js
+++ b/tools/eslint/node_modules/esrecurse/node_modules/estraverse/package.json
+++ b/tools/eslint/node_modules/estraverse/LICENSE.BSD
+++ b/tools/eslint/node_modules/esutils/LICENSE.BSD
+++ b/tools/eslint/node_modules/yocto-queue/license

@targos
Copy link
Member Author

targos commented Jun 13, 2024 •
edited
Loading

/cc @nodejs/linting

@aduh95
Copy link
Contributor

aduh95 commented Jun 13, 2024

Now we don't have too many added files:

$ git show eb0f9bcde1cad9fb76d2509162ce06c89bb54a93 --diff-filter=A | grep "+++ b"

note: you can use --name-only to ask git to show you only the name of files, instead of greping the whole diff

aduh95
aduh95 reviewed Jun 13, 2024
View reviewed changes
tools/eslint/package-lock.json Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to commit this file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to keep it for two reasons:

  • It allows to review what happened when an update PR is opened.
  • It will be possible to recover the exact dependency tree from the released tarball.

targos and others added 2 commits June 13, 2024 12:43
@targos targos mentioned this pull request Jun 16, 2024
Closed
aduh95
aduh95 approved these changes Jun 17, 2024
View reviewed changes
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

34f3770 LGTM, RSLGTM for the whole diff

@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@targos targos added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 19, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 19, 2024
@nodejs-github-bot nodejs-github-bot merged commit 2eff28f into nodejs:main Jun 19, 2024
54 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 2eff28f

@targos targos deleted the mv-eslint branch June 19, 2024 19:54
targos added a commit that referenced this pull request Jun 20, 2024
@targos
tools: move ESLint to tools/eslint
30fdd48 
Greatly simplify how ESLint and its plugins are installed.

PR-URL: #53413
Reviewed-By: Antoine du Hamel <[email protected]>
eliphazbouye pushed a commit to eliphazbouye/node that referenced this pull request Jun 20, 2024
@targos @eliphazbouye
tools: move ESLint to tools/eslint
ba99a3b 
Greatly simplify how ESLint and its plugins are installed.

PR-URL: nodejs#53413
Reviewed-By: Antoine du Hamel <[email protected]>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
@targos @bmeck
tools: move ESLint to tools/eslint
c98f029 
Greatly simplify how ESLint and its plugins are installed.

PR-URL: nodejs#53413
Reviewed-By: Antoine du Hamel <[email protected]>
@targos targos mentioned this pull request Jun 25, 2024
Merged
@Karliz24 Karliz24 mentioned this pull request Jul 9, 2024
Open
@marco-ippolito marco-ippolito added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduh95 aduh95 aduh95 approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@targos @nodejs-github-bot @aduh95 @marco-ippolito