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.

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

tools: add lint rule to keep primordials in ASCII order #52592

Merged
merged 5 commits into from
Apr 21, 2024

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Apr 19, 2024

For the sake on consistency, reduce noise in PR reviews, and to minimize the chance of conflicts if two PR tries to introduce the same primordial.
Given the size of the change, we've done an OK job at keeping the ASCII order manually, however there's no reason to that work manually when our computer can do it better than us :)

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/http
  • @nodejs/http2
  • @nodejs/loaders
  • @nodejs/net
  • @nodejs/startup
  • @nodejs/web-standards

@nodejs-github-bot 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. labels Apr 19, 2024
@anonrig
Copy link
Member

anonrig commented Apr 19, 2024

I wish we adopted organizeImports by Biome - https://biomejs.dev/analyzer/import-sorting

@joyeecheung
Copy link
Member

joyeecheung commented Apr 19, 2024

I wish we adopted organizeImports by Biome - https://biomejs.dev/analyzer/import-sorting

Note that primordials are not imported. They are carried around by the function contexts in the internal modules (which are not ESM, either, but wrapped functions similar to CJS)

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Apr 19, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 19, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot

This comment was marked as outdated.

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 20, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 21, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/52592
✔  Done loading data for nodejs/node/pull/52592
----------------------------------- PR info ------------------------------------
Title      tools: add lint rule to keep primordials in ASCII order (#52592)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     aduh95:primordials-ascii-order -> nodejs:main
Labels     lib / src, author ready, needs-ci
Commits    5
 - tools: add lint rule to keep primordials in ASCII order
 - fixup! tools: add lint rule to keep primordials in ASCII order
 - enforce multiline
 - fixup! enforce multiline
 - fixup! enforce multiline
Committers 1
 - Antoine du Hamel 
PR-URL: https://github.com/nodejs/node/pull/52592
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Jacob Smith 
Reviewed-By: Moshe Atlow 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/52592
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Jacob Smith 
Reviewed-By: Moshe Atlow 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 19 Apr 2024 14:12:46 GMT
   ✔  Approvals: 3
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/52592#pullrequestreview-2013069800
   ✔  - Jacob Smith (@JakobJingleheimer): https://github.com/nodejs/node/pull/52592#pullrequestreview-2011794306
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/52592#pullrequestreview-2013335800
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-04-20T10:58:00Z: https://ci.nodejs.org/job/node-test-pull-request/58543/
- Querying data for job/node-test-pull-request/58543/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 52592
From https://github.com/nodejs/node
 * branch                  refs/pull/52592/merge -> FETCH_HEAD
✔  Fetched commits as 461722d64cfd..a37d2682e6f6
--------------------------------------------------------------------------------
Auto-merging lib/internal/modules/esm/get_format.js
[main 8c2d7fc8ad] tools: add lint rule to keep primordials in ASCII order
 Author: Antoine du Hamel 
 Date: Fri Apr 19 16:06:52 2024 +0200
 87 files changed, 281 insertions(+), 180 deletions(-)
 create mode 100644 test/parallel/test-eslint-alphabetize-primordials.js
 create mode 100644 tools/eslint-rules/alphabetize-primordials.js
[main 85a6613c8b] fixup! tools: add lint rule to keep primordials in ASCII order
 Author: Antoine du Hamel 
 Date: Fri Apr 19 16:14:58 2024 +0200
 1 file changed, 1 insertion(+), 1 deletion(-)
[main cbb4bf5260] enforce multiline
 Author: Antoine du Hamel 
 Date: Fri Apr 19 17:27:10 2024 +0200
 2 files changed, 23 insertions(+), 9 deletions(-)
[main 7b4bc5cdc4] fixup! enforce multiline
 Author: Antoine du Hamel 
 Date: Fri Apr 19 17:33:36 2024 +0200
 10 files changed, 38 insertions(+), 10 deletions(-)
[main 53e4c0aaee] fixup! enforce multiline
 Author: Antoine du Hamel 
 Date: Fri Apr 19 18:39:23 2024 +0200
 1 file changed, 7 insertions(+), 3 deletions(-)
   ✔  Patches applied
There are 5 commits in the PR. Attempting autorebase.
Rebasing (2/7)
Rebasing (3/7)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
tools: add lint rule to keep primordials in ASCII order

PR-URL: #52592
Reviewed-By: Yagiz Nizipli [email protected]
Reviewed-By: Jacob Smith [email protected]
Reviewed-By: Moshe Atlow [email protected]

[detached HEAD bf98907e89] tools: add lint rule to keep primordials in ASCII order
Author: Antoine du Hamel [email protected]
Date: Fri Apr 19 16:06:52 2024 +0200
87 files changed, 281 insertions(+), 180 deletions(-)
create mode 100644 test/parallel/test-eslint-alphabetize-primordials.js
create mode 100644 tools/eslint-rules/alphabetize-primordials.js
Rebasing (4/7)
Rebasing (5/7)
Rebasing (6/7)
Rebasing (7/7)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
enforce multiline

PR-URL: #52592
Reviewed-By: Yagiz Nizipli [email protected]
Reviewed-By: Jacob Smith [email protected]
Reviewed-By: Moshe Atlow [email protected]

[detached HEAD 93bb80613d] enforce multiline
Author: Antoine du Hamel [email protected]
Date: Fri Apr 19 17:27:10 2024 +0200
12 files changed, 68 insertions(+), 22 deletions(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

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

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Apr 21, 2024
@aduh95 aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Apr 21, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 21, 2024
@nodejs-github-bot nodejs-github-bot merged commit a596af0 into nodejs:main Apr 21, 2024
64 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in a596af0

aduh95 added a commit that referenced this pull request Apr 29, 2024
PR-URL: #52592
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
@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 May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants