Skip to content

[v20.x backport] streams: fixes for webstreams#52773

Closed
MattiasBuelens wants to merge 1 commit into
nodejs:v20.x-stagingfrom
MattiasBuelens:backport-51168-to-v20.x
Closed

[v20.x backport] streams: fixes for webstreams#52773
MattiasBuelens wants to merge 1 commit into
nodejs:v20.x-stagingfrom
MattiasBuelens:backport-51168-to-v20.x

Conversation

@MattiasBuelens

@MattiasBuelens MattiasBuelens commented May 1, 2024

Copy link
Copy Markdown
Contributor

Manual backport of #51168 to v20.x.
Depends on #52772, so keeping this in draft until that one lands.

This one was a bit tricky, since there are changes to transferables on v22.x that are not on v20.x (such as #50107) which also affect web streams. However, I think I made it work. 🤞

@nodejs-github-bot

nodejs-github-bot commented May 1, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/web-standards

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. v20.x web streams labels May 1, 2024
@MattiasBuelens MattiasBuelens force-pushed the backport-51168-to-v20.x branch 2 times, most recently from 43c8f92 to a7758ee Compare May 1, 2024 11:18
@MattiasBuelens MattiasBuelens mentioned this pull request May 1, 2024
Merged
@marco-ippolito

marco-ippolito commented May 16, 2024

Copy link
Copy Markdown
Member

can you fix the conflicts? I'll try to backport it in the next v20 release

@MattiasBuelens MattiasBuelens force-pushed the backport-51168-to-v20.x branch from a7758ee to c6f0199 Compare May 16, 2024 18:27
@MattiasBuelens MattiasBuelens marked this pull request as ready for review May 16, 2024 18:27
@MattiasBuelens

MattiasBuelens commented May 16, 2024

Copy link
Copy Markdown
Contributor Author

@marco-ippolito Updated and ready for review. 😉

@marco-ippolito marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label May 17, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 17, 2024
@nodejs-github-bot

nodejs-github-bot commented May 17, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented May 17, 2024

Copy link
Copy Markdown
Collaborator

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

@marco-ippolito

marco-ippolito commented Jun 12, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Hi @MattiasBuelens I was backporting your commits, I encountered some issue, can you please cherry-pick the landed commits from the original PR (example #51168 (comment)), instead of the commits from your fork, so that they contain metadata like PR-URL and reviewers, and please squash the fixup commits

@marco-ippolito marco-ippolito force-pushed the v20.x-staging branch 3 times, most recently from a924e20 to 473fa73 Compare June 19, 2024 09:12
@MattiasBuelens MattiasBuelens force-pushed the backport-51168-to-v20.x branch from c6f0199 to 773f990 Compare June 25, 2024 19:09
@MattiasBuelens
stream: fixes for webstreams
573f5df 
PR-URL: nodejs#51168
Backport-PR-URL: nodejs#52773
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@MattiasBuelens MattiasBuelens force-pushed the backport-51168-to-v20.x branch from 773f990 to 573f5df Compare June 25, 2024 19:12
@MattiasBuelens

MattiasBuelens commented Jun 25, 2024

Copy link
Copy Markdown
Contributor Author

@marco-ippolito I've squashed the commits on this PR and rebased them onto v20.x-staging.

The original PR didn't have the most prominent change in the first commit, so we landed it in 20c6313 with the unimaginative title "stream: fix code style". 😅 I changed it to "stream: fixes for webstreams" for this PR, if that's okay.

@marco-ippolito

marco-ippolito commented Jun 25, 2024

Copy link
Copy Markdown
Member

@marco-ippolito I've squashed the commits on this PR and rebased them onto v20.x-staging.

The original PR didn't have the most prominent change in the first commit, so we landed it in 20c6313 with the unimaginative title "stream: fix code style". 😅 I changed it to "stream: fixes for webstreams" for this PR, if that's okay.

You can just cherry-pick the landed commits from the original PR (at the end of the pr in the comment from the bot landed in ...) and squash only the fixup commits

mcollina
mcollina approved these changes Jul 1, 2024
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
@MattiasBuelens @marco-ippolito
stream: fixes for webstreams
48138af 
PR-URL: #51168
Backport-PR-URL: #52773
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@marco-ippolito

marco-ippolito commented Jul 19, 2024

Copy link
Copy Markdown
Member

Landed in 48138af

@MattiasBuelens MattiasBuelens deleted the backport-51168-to-v20.x branch July 25, 2024 16:25
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

@marco-ippolito marco-ippolito marco-ippolito approved these changes

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. web streams

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MattiasBuelens @nodejs-github-bot @marco-ippolito @mcollina