Skip to content

Conversation

@debadree25
Copy link
Member

In our original PR we hadn't added support for AbortSignal when using webstreams with finished() hence added the same here

Refs: #46205
Refs: #37354

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Jan 29, 2023
@debadree25debadree25 changed the title stream: add supoort for abort signal in finished() for webstreamsstream: add suport for abort signal in finished() for webstreamsJan 29, 2023
@debadree25debadree25force-pushed the ft/abort-signal-webstreams-finish branch from eb31a9c to 4312987CompareJanuary 29, 2023 05:29
Copy link
Member

@mcollinamcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollinamcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 30, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 30, 2023
@nodejs-github-bot
Copy link
Collaborator

@lpincalpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 2, 2023
@nodejs-github-botnodejs-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 Feb 2, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/46403 ✔ Done loading data for nodejs/node/pull/46403 ----------------------------------- PR info ------------------------------------ Title stream: add suport for abort signal in finished() for webstreams (#46403) Author Debadree Chatterjee (@debadree25) Branch debadree25:ft/abort-signal-webstreams-finish -> nodejs:main Labels needs-ci Commits 2 - stream: add suport for abort signal in finished() for webstreams - fixup! add tests Committers 1 - Debadree Chatterjee PR-URL: https://github.com/nodejs/node/pull/46403 Refs: https://github.com/nodejs/node/pull/46205 Refs: https://github.com/nodejs/node/pull/37354 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: Robert Nagy ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/46403 Refs: https://github.com/nodejs/node/pull/46205 Refs: https://github.com/nodejs/node/pull/37354 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: Robert Nagy -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 29 Jan 2023 05:27:07 GMT ✔ Approvals: 3 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/46403#pullrequestreview-1274265207 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/46403#pullrequestreview-1274266823 ✔ - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/46403#pullrequestreview-1274274475 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-01-30T11:22:45Z: https://ci.nodejs.org/job/node-test-pull-request/49247/ - Querying data for job/node-test-pull-request/49247/ ✔ 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 f84de0ad4c..75080830a4 main -> origin/main ✔ origin/main is now up-to-date main is out of sync with origin/main. Mismatched commits: - 1f708d2037 stream: dont access Object.prototype.type during TransformStream init - 75080830a4 stream: dont access Object.prototype.type during TransformStream init -------------------------------------------------------------------------------- HEAD is now at 75080830a4 stream: dont access Object.prototype.type during TransformStream init ✔ Reset to origin/main - Downloading patch for 46403 From https://github.com/nodejs/node * branch refs/pull/46403/merge -> FETCH_HEAD ✔ Fetched commits as 75080830a40d..43129876236c -------------------------------------------------------------------------------- [main 3cbb426140] stream: add suport for abort signal in finished() for webstreams Author: Debadree Chatterjee Date: Sun Jan 29 09:48:41 2023 +0530 1 file changed, 26 insertions(+), 3 deletions(-) [main 3034793ca3] fixup! add tests Author: Debadree Chatterjee Date: Sun Jan 29 10:52:07 2023 +0530 1 file changed, 90 insertions(+) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. Rebasing (2/4) 

Executing: git node land --amend --yes
⚠ Found Refs: #46205, skipping..
--------------------------------- New Message ----------------------------------
stream: add suport for abort signal in finished() for webstreams

Refs: #46205
PR-URL: #46403
Refs: #37354
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Robert Nagy [email protected]

[detached HEAD b6ab6bb94a] stream: add suport for abort signal in finished() for webstreams
Author: Debadree Chatterjee [email protected]
Date: Sun Jan 29 09:48:41 2023 +0530
1 file changed, 26 insertions(+), 3 deletions(-)
Rebasing (3/4)
Rebasing (4/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
fixup! add tests

PR-URL: #46403
Refs: #46205
Refs: #37354
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Robert Nagy [email protected]

[detached HEAD f19ef3aaa8] fixup! add tests
Author: Debadree Chatterjee [email protected]
Date: Sun Jan 29 10:52:07 2023 +0530
1 file changed, 90 insertions(+)

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

@lpincalpinca added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. 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 Feb 2, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 2, 2023
@nodejs-github-botnodejs-github-bot merged commit ebcc711 into nodejs:mainFeb 2, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in ebcc711

MylesBorins pushed a commit that referenced this pull request Feb 18, 2023
Refs: #46205 PR-URL: #46403 Refs: #37354 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Feb 19, 2023
debadree25 added a commit to debadree25/node that referenced this pull request Feb 27, 2023
Refs: nodejs#46205 PR-URL: nodejs#46403 Refs: nodejs#37354 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
debadree25 added a commit to debadree25/node that referenced this pull request Feb 27, 2023
Refs: nodejs#46205 PR-URL: nodejs#46403 Refs: nodejs#37354 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
debadree25 added a commit to debadree25/node that referenced this pull request Feb 27, 2023
Refs: nodejs#46205 PR-URL: nodejs#46403 Refs: nodejs#37354 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
danielleadams pushed a commit that referenced this pull request Apr 11, 2023
Refs: #46205 PR-URL: #46403 Refs: #37354 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@debadree25@nodejs-github-bot@mcollina@benjamingr@ronag@lpinca