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

http, readline: replace sort with toSorted #53623

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

benjamingr
Copy link
Member

Just a minor chore, Richard's set.difference PR reminded me our V8 is new enough to support toSorted and we can avoid .slice().sort(...).

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. readline Issues and PRs related to the built-in readline module. typings labels Jun 28, 2024
@richardlau richardlau added the dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. label Jun 28, 2024
@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 Jun 28, 2024
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

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

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@mcollina
Copy link
Member

mcollina commented Jul 8, 2024

@benjamingr this now conflicts

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

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

This failed in the Jenkins CI because of the merge commit.

@benjamingr
Copy link
Member Author

Sorry, new job, new gitconfig, pushing a fix

@benjamingr benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 9, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 9, 2024
Copy link
Contributor

github-actions bot commented Jul 9, 2024

Failed to start CI
   ⚠  Something was pushed to the Pull Request branch since the last approving review.
   ✘  Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/9862771244

@github-actions github-actions bot added the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label Jul 9, 2024
@benjamingr benjamingr added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Jul 9, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 9, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@benjamingr benjamingr added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 10, 2024
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 10, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/53623
✔  Done loading data for nodejs/node/pull/53623
----------------------------------- PR info ------------------------------------
Title      http, readline: replace sort with toSorted (#53623)
Author     Benjamin Gruenbaum <inglor@gmail.com> (@benjamingr)
Branch     benjamingr:replace-with-tosorted -> nodejs:main
Labels     http, readline, needs-ci, typings, dont-land-on-v18.x, dont-land-on-v20.x
Commits    1
 - http, readline: replace sort with toSorted
Committers 1
 - Benjamin Gruenbaum <benji@eon.io>
PR-URL: https://github.com/nodejs/node/pull/53623
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/53623
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - http, readline: replace sort with toSorted
   ℹ  This PR was created on Fri, 28 Jun 2024 12:38:56 GMT
   ✔  Approvals: 8
   ✔  - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/53623#pullrequestreview-2148102817
   ✔  - Richard Lau (@richardlau) (TSC): https://github.com/nodejs/node/pull/53623#pullrequestreview-2148120155
   ✔  - Tim Perry (@pimterry): https://github.com/nodejs/node/pull/53623#pullrequestreview-2148213278
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/53623#pullrequestreview-2148271807
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/53623#pullrequestreview-2149963447
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/53623#pullrequestreview-2149970876
   ✔  - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/53623#pullrequestreview-2150175739
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/53623#pullrequestreview-2151991049
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-07-09T23:43:34Z: https://ci.nodejs.org/job/node-test-pull-request/60208/
- Querying data for job/node-test-pull-request/60208/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/9877917825

@richardlau richardlau added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jul 10, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 10, 2024
@nodejs-github-bot nodejs-github-bot merged commit fd21dd4 into nodejs:main Jul 10, 2024
61 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in fd21dd4

@benjamingr benjamingr deleted the replace-with-tosorted branch July 10, 2024 17:35
aduh95 pushed a commit that referenced this pull request Jul 12, 2024
PR-URL: #53623
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@aduh95 aduh95 mentioned this pull request Jul 12, 2024
aduh95 pushed a commit that referenced this pull request Jul 16, 2024
PR-URL: #53623
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
ehsankhfr pushed a commit to ehsankhfr/node that referenced this pull request Jul 18, 2024
PR-URL: nodejs#53623
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. readline Issues and PRs related to the built-in readline module. typings
Projects
None yet
Development

Successfully merging this pull request may close these issues.