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

[v13.x backport] stream: support passing generator functions into pipeline() by ronag · Pull Request #31975 · nodejs/node · GitHub

/ node Public

[v13.x backport] stream: support passing generator functions into pipeline() - #31975

Closed
ronag wants to merge 3 commits into
nodejs:v13.x-stagingfrom
nxtedition:backport-pipeline-generator
ConversationCommits (3)ChecksFiles changed
Closed

[v13.x backport] stream: support passing generator functions into pipeline()#31975
ronag wants to merge 3 commits into
nodejs:v13.x-stagingfrom
nxtedition:backport-pipeline-generator

Conversation

ronag commented Feb 26, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

#31223

Also includes some required follow up fixes.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

nodejs-github-bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. v13.x labels Feb 26, 2020

ronag commented Feb 26, 2020

Copy link
Copy Markdown
Member Author

Since this backport contains multiple commits I'm unsure where to add the Backport-PR-URL: meta. Should it be on every commit?

ronag requested a review from codebytere February 26, 2020 22:54

nodejs-github-bot commented Feb 26, 2020

Copy link
Copy Markdown
Collaborator

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

ronag added stream Issues and PRs related to the stream subsystem. notable-change PRs with changes that should be highlighted in changelogs. and removed tools Issues and PRs related to the tools directory. labels Feb 26, 2020
ronag requested a review from mcollina February 26, 2020 23:17
ronag added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. and removed notable-change PRs with changes that should be highlighted in changelogs. labels Feb 26, 2020
codebytere approved these changes Feb 27, 2020
View reviewed changes

codebytere left a comment •
edited
Loading

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

🙇‍♀ ty - and yes @ronag every commit. I'll get this merged as soon as that's been updated as long as @mcollina has no concerns!

ronag added 3 commits February 28, 2020 00:24
stream: support passing generator functions into pipeline()
8af1387 
Backport-PR-URL: nodejs#31975
PR-URL: nodejs#31223
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
stream: fix broken pipeline error propagation
21a0dd0 
If the destination was an async function any
error thrown from that function would be swallowed.

Backport-PR-URL: nodejs#31975
PR-URL: nodejs#31835
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
stream: ensure pipeline always destroys streams
23011b9 
There was an edge case where an incorrect assumption was made
in regardos whether eos/finished means that the stream is
actually destroyed or not.

Backport-PR-URL: nodejs#31975
PR-URL: nodejs#31940
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
ronag force-pushed the backport-pipeline-generator branch from 995dcc6 to 23011b9 Compare February 27, 2020 23:25

ronag commented Feb 27, 2020

Copy link
Copy Markdown
Member Author

Added Backport-PR-URL to each commit

codebytere commented Feb 29, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

@ronag each commit also needs the normal PR-URL pointing to the initial PR

(update, nevermind - this looks like a bug in ncu landing behavior!)

codebytere commented Feb 29, 2020

Copy link
Copy Markdown
Member

@nodejs/releasers would someone else be willing to review this? with only 1 approval the required waiting period is 97 more hours and I'd like to finish preparing the release unless this can be left off it. It does, however, block clean landing of ~10 other streams backports.

Trott approved these changes Feb 29, 2020
View reviewed changes

Trott 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

A little bit rubber-stampy of me but this looks good to me.

codebytere mentioned this pull request Feb 29, 2020
Merged
codebytere pushed a commit that referenced this pull request Mar 1, 2020
stream: support passing generator functions into pipeline()
8ad64b8 
Backport-PR-URL: #31975
PR-URL: #31223
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
codebytere pushed a commit that referenced this pull request Mar 1, 2020
stream: fix broken pipeline error propagation
313ecaa 
If the destination was an async function any
error thrown from that function would be swallowed.

Backport-PR-URL: #31975
PR-URL: #31835
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
codebytere pushed a commit that referenced this pull request Mar 1, 2020
stream: ensure pipeline always destroys streams
8a2b62e 
There was an edge case where an incorrect assumption was made
in regardos whether eos/finished means that the stream is
actually destroyed or not.

Backport-PR-URL: #31975
PR-URL: #31940
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

codebytere commented Mar 1, 2020 •
edited by targos
Loading

Copy link
Copy Markdown
Member

Landed in 8ad64b8...8a2b62e

codebytere closed this Mar 1, 2020
ronag mentioned this pull request Mar 9, 2020
Closed
4 tasks
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

Trott Trott approved these changes
codebytere codebytere approved these changes
mcollina Awaiting requested review from mcollina

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL