Skip to content

doc: add pipeline and finished to stream/promises doc#45832

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.

Sign up for GitHub

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

Jump to bottom
Merged
nodejs-github-bot merged 8 commits into from
Dec 15, 2022
Merged

doc: add pipeline and finished to stream/promises doc #45832

nodejs-github-bot merged 8 commits into from
Dec 15, 2022

Conversation

@marco-ippolito
Copy link
Member

@marco-ippolitomarco-ippolito commented Dec 12, 2022•
edited by aduh95
Loading

I've added to the doc the signature of pipeline and finished which was missing.
@ronag I've also added the end field which was mentioned here #34805 (comment) but I'm not sure if that's all right.

Fixes: #45821

@nodejs-github-botnodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. labels Dec 12, 2022
@marco-ippolitomarco-ippolito changed the title streams: add pipeline and finished to stream/promises docdoc: add pipeline and finished to stream/promises docDec 12, 2022
@marco-ippolitomarco-ippolitoforce-pushed the doc/stream-promises branch 3 times, most recently from acb6aa0 to 0eee704CompareDecember 12, 2022 18:59
aduh95
aduh95 reviewed Dec 13, 2022
View reviewed changes
Copy link
Contributor

@aduh95aduh95 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! Can you add back link to the callback methods and add link this this methods from the callback docs? Adding code examples would be great also.

@marco-ippolito
Copy link
MemberAuthor

marco-ippolito commented Dec 13, 2022

I've moved the async examples under the new async documentation and created links

aduh95
aduh95 reviewed Dec 14, 2022
View reviewed changes
Copy link
Contributor

@aduh95aduh95 left a comment

Choose a reason for hiding this comment

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

Looks good. Let's use top-level await on ESM when possible.

aduh95
aduh95 approved these changes Dec 15, 2022
View reviewed changes
@marco-ippolito@aduh95
doc: fix setImmediate
083825e
Co-authored-by: Antoine du Hamel <[email protected]>
@aduh95aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 15, 2022
@nodejs-github-botnodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 15, 2022
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 15, 2022

Commit Queue failed
- Loading data for nodejs/node/pull/45832 ✔ Done loading data for nodejs/node/pull/45832 ----------------------------------- PR info ------------------------------------ Title doc: add pipeline and finished to stream/promises doc (#45832) Author Marco Ippolito (@marco-ippolito) Branch marco-ippolito:doc/stream-promises -> nodejs:main Labels author ready, needs-ci, dependencies, commit-queue-squash Commits 6 - doc: add stream/promises pipeline and finished to doc - doc: fixed correct return type - doc: improved documentation - doc: typo removed > - doc: top level await and typo - doc: fix setImmediate Committers 2 - Marco Ippolito - GitHub PR-URL: https://github.com/nodejs/node/pull/45832 Fixes: https://github.com/nodejs/node/issues/45821 Reviewed-By: Paolo Insogna Reviewed-By: Antoine du Hamel ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/45832 Fixes: https://github.com/nodejs/node/issues/45821 Reviewed-By: Paolo Insogna Reviewed-By: Antoine du Hamel -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last review: ⚠ - doc: fix setImmediate ℹ This PR was created on Mon, 12 Dec 2022 18:51:35 GMT ✔ Approvals: 2 ✔ - Paolo Insogna (@ShogunPanda): https://github.com/nodejs/node/pull/45832#pullrequestreview-1217021378 ✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/45832#pullrequestreview-1218851209 ✔ Last GitHub CI successful ✖ No Jenkins CI runs detected -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/3703786198

aduh95
aduh95 reviewed Dec 15, 2022
View reviewed changes
marco-ippolitoand others added 2 commits December 15, 2022 14:03
@marco-ippolito@aduh95
fix: period instead of :
50bf959
Co-authored-by: Antoine du Hamel <[email protected]>
@marco-ippolito@aduh95
doc: newline
c930116
Co-authored-by: Antoine du Hamel <[email protected]>
@aduh95
Copy link
Contributor

aduh95 commented Dec 15, 2022

Not sure why GitHub didn't run CI on the lastest commit, closing and reopening to trigger CI.

@aduh95aduh95 closed this Dec 15, 2022
@aduh95aduh95 reopened this Dec 15, 2022
@aduh95aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Dec 15, 2022
@nodejs-github-botnodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 15, 2022
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 15, 2022

Commit Queue failed
- Loading data for nodejs/node/pull/45832 ✔ Done loading data for nodejs/node/pull/45832 ----------------------------------- PR info ------------------------------------ Title doc: add pipeline and finished to stream/promises doc (#45832) Author Marco Ippolito (@marco-ippolito) Branch marco-ippolito:doc/stream-promises -> nodejs:main Labels author ready, needs-ci, dependencies, commit-queue-squash Commits 8 - doc: add stream/promises pipeline and finished to doc - doc: fixed correct return type - doc: improved documentation - doc: typo removed > - doc: top level await and typo - doc: fix setImmediate - fix: period instead of : - doc: newline Committers 2 - Marco Ippolito - GitHub PR-URL: https://github.com/nodejs/node/pull/45832 Fixes: https://github.com/nodejs/node/issues/45821 Reviewed-By: Paolo Insogna Reviewed-By: Antoine du Hamel ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/45832 Fixes: https://github.com/nodejs/node/issues/45821 Reviewed-By: Paolo Insogna Reviewed-By: Antoine du Hamel -------------------------------------------------------------------------------- ℹ This PR was created on Mon, 12 Dec 2022 18:51:35 GMT ✔ Approvals: 2 ✔ - Paolo Insogna (@ShogunPanda): https://github.com/nodejs/node/pull/45832#pullrequestreview-1217021378 ✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/45832#pullrequestreview-1219295708 ✔ Last GitHub CI successful ✖ No Jenkins CI runs detected -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/3705387087

@aduh95aduh95 added doc Issues and PRs related to the documentations. commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. dependencies Pull requests that update a dependency file. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Dec 15, 2022
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 15, 2022
@nodejs-github-botnodejs-github-bot merged commit 4166d40 into nodejs:mainDec 15, 2022
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 15, 2022

Landed in 4166d40

targos pushed a commit that referenced this pull request Jan 1, 2023
@marco-ippolito@targos
doc: add stream/promises pipeline and finished to doc
b93a967
PR-URL: #45832Fixes: #45821 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
@RafaelGSSRafaelGSS mentioned this pull request Jan 2, 2023
Merged
juanarbol pushed a commit that referenced this pull request Jan 26, 2023
@marco-ippolito@juanarbol
doc: add stream/promises pipeline and finished to doc
5eb93f1
PR-URL: #45832Fixes: #45821 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
@juanarboljuanarbol mentioned this pull request Jan 28, 2023
Merged
@aloisklinkaloisklink mentioned this pull request Jul 30, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ShogunPandaShogunPandaShogunPanda approved these changes

@aduh95aduh95aduh95 approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.docIssues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

The stream/promises API is poorly documented

4 participants

@marco-ippolito@nodejs-github-bot@aduh95@ShogunPanda