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

dgram, net: support Symbol.asyncDispose #48717

Closed
wants to merge 2 commits into from

Conversation

atlowChemi
Copy link
Member

No description provided.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added dgram Issues and PRs related to the dgram subsystem / UDP. needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem. labels Jul 9, 2023
lib/dgram.js Show resolved Hide resolved
lib/net.js Show resolved Hide resolved
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

@atlowChemi atlowChemi 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 Jul 10, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 10, 2023
@nodejs-github-bot
Copy link
Collaborator

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

nodejs-github-bot commented Jul 11, 2023

@atlowChemi atlowChemi added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 11, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 11, 2023
@MoLow
Copy link
Member

MoLow commented Jul 11, 2023

@atlowChemi asked me where is the comment of the CI,
so here it is: https://ci.nodejs.org/job/node-test-pull-request/52701/

is seems like we passed the rate limit of the github api

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

Commit Queue failed
- Loading data for nodejs/node/pull/48717
✔  Done loading data for nodejs/node/pull/48717
----------------------------------- PR info ------------------------------------
Title      dgram, net: support `Symbol.asyncDispose` (#48717)
Author     Chemi Atlow  (@atlowChemi)
Branch     atlowChemi:dgram-asyncDispose -> nodejs:main
Labels     dgram, net, author ready, needs-ci
Commits    2
 - dgram: socket add `asyncDispose`
 - net: server add `asyncDispose`
Committers 1
 - atlowChemi 
PR-URL: https://github.com/nodejs/node/pull/48717
Reviewed-By: Matteo Collina 
Reviewed-By: Moshe Atlow 
Reviewed-By: Benjamin Gruenbaum 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/48717
Reviewed-By: Matteo Collina 
Reviewed-By: Moshe Atlow 
Reviewed-By: Benjamin Gruenbaum 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 09 Jul 2023 19:34:20 GMT
   ✔  Approvals: 3
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/48717#pullrequestreview-1520959501
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/48717#pullrequestreview-1524942718
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/48717#pullrequestreview-1524737638
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-07-11T18:34:30Z: https://ci.nodejs.org/job/node-test-pull-request/52701/
- Querying data for job/node-test-pull-request/52701/
   ✔  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 48717
From https://github.com/nodejs/node
 * branch                  refs/pull/48717/merge -> FETCH_HEAD
✔  Fetched commits as ccdfb374383a..4792bdf0582d
--------------------------------------------------------------------------------
[main 20437fb365] dgram: socket add `asyncDispose`
 Author: atlowChemi 
 Date: Mon Jul 10 00:21:58 2023 +0300
 3 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-dgram-async-dispose.mjs
[main a31100b984] net: server add `asyncDispose`
 Author: atlowChemi 
 Date: Mon Jul 10 00:22:05 2023 +0300
 3 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-net-server-async-dispose.mjs
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
dgram: socket add asyncDispose

PR-URL: #48717
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Moshe Atlow [email protected]
Reviewed-By: Benjamin Gruenbaum [email protected]

[detached HEAD 11447cd8e4] dgram: socket add asyncDispose
Author: atlowChemi [email protected]
Date: Mon Jul 10 00:21:58 2023 +0300
3 files changed, 41 insertions(+), 1 deletion(-)
create mode 100644 test/parallel/test-dgram-async-dispose.mjs
Rebasing (3/4)
Rebasing (4/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
net: server add asyncDispose

PR-URL: #48717
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Moshe Atlow [email protected]
Reviewed-By: Benjamin Gruenbaum [email protected]

[detached HEAD c1e9dc6ce0] net: server add asyncDispose
Author: atlowChemi [email protected]
Date: Mon Jul 10 00:22:05 2023 +0300
3 files changed, 51 insertions(+), 1 deletion(-)
create mode 100644 test/parallel/test-net-server-async-dispose.mjs

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/5524812627

@atlowChemi atlowChemi added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jul 11, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 11, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in ccdfb37...0e9138d

nodejs-github-bot pushed a commit that referenced this pull request Jul 11, 2023
PR-URL: #48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Jul 11, 2023
PR-URL: #48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
juanarbol pushed a commit that referenced this pull request Jul 13, 2023
PR-URL: #48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
juanarbol pushed a commit that referenced this pull request Jul 13, 2023
PR-URL: #48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@juanarbol juanarbol mentioned this pull request Jul 13, 2023
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
PR-URL: nodejs#48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
PR-URL: nodejs#48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
PR-URL: nodejs#48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
PR-URL: nodejs#48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
ruyadorno pushed a commit that referenced this pull request Sep 11, 2023
PR-URL: #48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
ruyadorno pushed a commit that referenced this pull request Sep 11, 2023
PR-URL: #48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Sep 11, 2023
ruyadorno pushed a commit that referenced this pull request Sep 13, 2023
PR-URL: #48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
ruyadorno pushed a commit that referenced this pull request Sep 13, 2023
PR-URL: #48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
ruyadorno pushed a commit that referenced this pull request Sep 17, 2023
PR-URL: #48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
ruyadorno pushed a commit that referenced this pull request Sep 17, 2023
PR-URL: #48717
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
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-rebase Add this label to allow the Commit Queue to land a PR in several commits. dgram Issues and PRs related to the dgram subsystem / UDP. needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants