FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

stream: make Duplex inherit destroy from Writable by lpinca · Pull Request #52318 · nodejs/node · GitHub

/ node Public

stream: make Duplex inherit destroy from Writable - #52318

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
lpinca:fix/duplex-destroy
Apr 5, 2024
Merged

stream: make Duplex inherit destroy from Writable#52318
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
lpinca:fix/duplex-destroy

Conversation

lpinca commented Apr 2, 2024
edited
Loading

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 2, 2024
lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 2, 2024

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 2, 2024

Copy link
Copy Markdown
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 force-pushed the fix/duplex-destroy branch from ce36309 to ea1b2d8 Compare April 2, 2024 06:30
lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 2, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 2, 2024

Copy link
Copy Markdown
Collaborator

ronag 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Good find!

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 5, 2024
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 merged commit c02de65 into nodejs:main Apr 5, 2024

Copy link
Copy Markdown
Collaborator

Landed in c02de65

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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL