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

stream: make Duplex inherit destroy from Writable #52318

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

lpinca
Copy link
Member

@lpinca lpinca commented Apr 2, 2024

Make Duplex inherit the destroy method from Writable instead of Readable so that pending write callbacks are correctly invoked when the stream is destroyed.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 2, 2024
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 2, 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

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

Make `Duplex` inherit the `destroy` method from `Writable` instead of
`Readable` so that pending write callbacks are correctly invoked when
the stream is destroyed.
@lpinca lpinca force-pushed the fix/duplex-destroy branch from ce36309 to ea1b2d8 Compare April 2, 2024 06:30
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 2, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 2, 2024
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@ronag ronag left a comment

Choose a reason for hiding this comment

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

Good find!

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 5, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 5, 2024
@nodejs-github-bot nodejs-github-bot merged commit c02de65 into nodejs:main Apr 5, 2024
56 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in c02de65

@lpinca lpinca deleted the fix/duplex-destroy branch April 5, 2024 18:35
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
Make `Duplex` inherit the `destroy` method from `Writable` instead of
`Readable` so that pending write callbacks are correctly invoked when
the stream is destroyed.

PR-URL: #52318
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
Make `Duplex` inherit the `destroy` method from `Writable` instead of
`Readable` so that pending write callbacks are correctly invoked when
the stream is destroyed.

PR-URL: #52318
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants