GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — stream: add suport for abort signal in finished() for webstreams by debadree25 · Pull Request #46403 · nodejs/node

Conversation

@debadree25

@debadree25 debadree25 commented Jan 29, 2023

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss and tonivj5 reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@nodejs-github-bot

nodejs-github-bot commented Jan 29, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jan 29, 2023
@debadree25 debadree25 changed the title stream: add supoort for abort signal in finished() for webstreams stream: add suport for abort signal in finished() for webstreams Jan 29, 2023
debadree25 added 2 commits January 29, 2023 10:59
@debadree25
stream: add suport for abort signal in finished() for webstreams
ee41163
Refs: nodejs#46205
@debadree25
fixup! add tests
4312987
@debadree25
debadree25 force-pushed the ft/abort-signal-webstreams-finish branch from eb31a9c to 4312987 Compare January 29, 2023 05:29
benjamingr
benjamingr approved these changes Jan 29, 2023
View reviewed changes
mcollina
mcollina approved these changes Jan 29, 2023
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
ronag
ronag approved these changes Jan 29, 2023
View reviewed changes
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jan 30, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jan 30, 2023
@nodejs-github-bot

nodejs-github-bot commented Jan 30, 2023

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@lpinca lpinca added the commit-queue PRs queued for automated landing through the Commit Queue. label Feb 2, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Feb 2, 2023
@nodejs-github-bot

nodejs-github-bot commented Feb 2, 2023

Copy link
Copy Markdown
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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@lpinca lpinca added commit-queue-squash PRs the Commit Queue should land as one squashed commit. commit-queue PRs queued for automated landing through the Commit Queue. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels Feb 2, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Feb 2, 2023
@nodejs-github-bot
nodejs-github-bot merged commit ebcc711 into nodejs:main Feb 2, 2023
@nodejs-github-bot

nodejs-github-bot commented Feb 2, 2023

Copy link
Copy Markdown
Collaborator

Landed in ebcc711

debadree25 reacted with rocket emoji
All reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MylesBorins pushed a commit that referenced this pull request Feb 18, 2023
@debadree25 @MylesBorins
stream: add suport for abort signal in finished() for webstreams
4cf4b41
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]>
@MylesBorins MylesBorins mentioned this pull request Feb 19, 2023
v19.7.0 proposal #46725
Merged
debadree25 added a commit to debadree25/node that referenced this pull request Feb 27, 2023
@debadree25
stream: add suport for abort signal in finished() for webstreams
f141b73
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 debadree25 mentioned this pull request Feb 27, 2023
[v18.x backport] stream: streams and webstreams interop related changes #46314
Closed
danielleadams pushed a commit that referenced this pull request Apr 11, 2023
@debadree25 @danielleadams
stream: add suport for abort signal in finished() for webstreams
91a550e
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]>
@danielleadams danielleadams mentioned this pull request Apr 11, 2023
v18.16.0 release proposal #47502
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes
@benjamingr benjamingr benjamingr approved these changes
@ronag ronag ronag approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

commit-queue-squash PRs the Commit Queue should land as one squashed commit. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@debadree25 @nodejs-github-bot @mcollina @benjamingr @ronag @lpinca
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.