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

stream: fix Transform with hwm 0 regression by kanongil · Pull Request #43648 · nodejs/node · GitHub

/ node Public

stream: fix Transform with hwm 0 regression - #43648

Closed
kanongil wants to merge 1 commit into
nodejs:mainfrom
kanongil:fix-transform-hwm0
ConversationCommits (1)ChecksFiles changed
Closed

stream: fix Transform with hwm 0 regression#43648
kanongil wants to merge 1 commit into
nodejs:mainfrom
kanongil:fix-transform-hwm0

Conversation

kanongil commented Jul 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

This undoes a patch from #40947, and reworks it to continue handling backpressure when highWaterMark: 0, as described in #42457 (comment).

The new test fails on node releases with the patch from #40947, like v16.4, and passes on node v14 and node v16.3 from before it was introduced. It is also designed, so that it will fail if highWaterMark: 1 is set instead.

nodejs-github-bot commented Jul 1, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jul 1, 2022
kanongil mentioned this pull request Jul 1, 2022
Closed
kanongil force-pushed the fix-transform-hwm0 branch from 6c98049 to b8a1698 Compare July 1, 2022 20:01
mcollina approved these changes Jul 1, 2022
View reviewed changes

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

mcollina requested a review from ronag July 1, 2022 23:03
aduh95 added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jul 5, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 5, 2022
mcollina requested a review from ShogunPanda July 5, 2022 08:21

ronag commented Jul 5, 2022

Copy link
Copy Markdown
Member

Looking at this now. I agree the hwm === 0 check is incorrect. Not sure about the fix though. Digging into it.

ShogunPanda approved these changes Jul 5, 2022
View reviewed changes

ShogunPanda left a comment

Copy link
Copy Markdown
Contributor

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!

Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 5, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 5, 2022

ronag commented Jul 6, 2022

Copy link
Copy Markdown
Member

@kanongil Could you rename this PR or create a new one for lazy reading?

aduh95 removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jul 9, 2022

aduh95 commented Jul 9, 2022

Copy link
Copy Markdown
Contributor

Removing author ready as it looks #43648 (comment) needs to be addressed. @ronag please re-add the label if I misinterpreted your comment.

aduh95 added the stalled Issues and PRs that are stalled. label May 11, 2024

github-actions Bot commented May 11, 2024

Copy link
Copy Markdown
Contributor

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

github-actions Bot commented Jun 12, 2024

Copy link
Copy Markdown
Contributor

Closing this because it has stalled. Feel free to reopen if this issue/PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions.

github-actions Bot closed this Jun 12, 2024
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina approved these changes
ShogunPanda ShogunPanda approved these changes
ronag Awaiting requested review from ronag

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. stalled Issues and PRs that are stalled.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL