Skip to content

doc: stream.finished cleanup#28935

Closed
ronag wants to merge 2 commits intonodejs:masterfrom
nxtedition:doc-finished-cleanup
Closed

doc: stream.finished cleanup#28935
ronag wants to merge 2 commits intonodejs:masterfrom
nxtedition:doc-finished-cleanup

Conversation

@ronag
Copy link
Member

@ronag ronag commented Aug 2, 2019

Documents stream.finished cleanup behaviour.

Checklist

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. labels Aug 2, 2019
@ronag ronag mentioned this pull request Aug 2, 2019
Closed
4 tasks
@ronag ronag changed the title docs: stream.finished cleanup doc: stream.finished cleanup Aug 2, 2019
@ronag ronag force-pushed the doc-finished-cleanup branch 3 times, most recently from db172c7 to 12448b1 Compare August 2, 2019 07:04
mcollina
mcollina reviewed Aug 2, 2019
View reviewed changes
@ronag ronag force-pushed the doc-finished-cleanup branch 2 times, most recently from 7b2d4af to 07f5751 Compare August 2, 2019 07:14
vsemozhetbyt
vsemozhetbyt reviewed Aug 2, 2019
View reviewed changes
vsemozhetbyt
vsemozhetbyt reviewed Aug 2, 2019
View reviewed changes
vsemozhetbyt
vsemozhetbyt reviewed Aug 2, 2019
View reviewed changes
vsemozhetbyt
vsemozhetbyt reviewed Aug 2, 2019
View reviewed changes
vsemozhetbyt
vsemozhetbyt reviewed Aug 2, 2019
View reviewed changes
@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Aug 2, 2019

Just a nit: it seems "note that" is avoided in the docs recently, see #28329, #28327, #21646

@ronag ronag force-pushed the doc-finished-cleanup branch 2 times, most recently from 4552345 to f049f17 Compare August 2, 2019 17:01
Trott
Trott reviewed Aug 2, 2019
View reviewed changes
Trott
Trott reviewed Aug 2, 2019
View reviewed changes
Trott
Trott reviewed Aug 2, 2019
View reviewed changes
Trott
Trott reviewed Aug 2, 2019
View reviewed changes
doc/api/stream.md Outdated
Copy link
Member

@Trott Trott Aug 2, 2019

Choose a reason for hiding this comment

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

Unrelated whitespace change?

Copy link
Member

@mcollina mcollina Aug 3, 2019

Choose a reason for hiding this comment

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

@ronag ^^

Trott
Trott approved these changes Aug 2, 2019
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM assuming nits/suggestions are applied.

@ronag ronag force-pushed the doc-finished-cleanup branch 2 times, most recently from afd8eff to 264bb84 Compare August 3, 2019 10:15
@ronag ronag force-pushed the doc-finished-cleanup branch from 264bb84 to d3cdaed Compare August 3, 2019 14:49
This was referenced Aug 6, 2019
Closed
Closed
@Trott
Copy link
Member

Trott commented Aug 20, 2019

Rebase to remove conflict?

@Trott
Copy link
Member

Trott commented Aug 20, 2019

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3838/

@Trott Trott added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Aug 20, 2019
@ronag
Copy link
Member Author

ronag commented Aug 23, 2019

Fixed travis lint failure.

@ronag ronag force-pushed the doc-finished-cleanup branch from f1c83b2 to 462c37e Compare August 23, 2019 07:43
@ronag
Copy link
Member Author

ronag commented Sep 18, 2019

@Trott: Is this missing anything?

@Trott
Copy link
Member

Trott commented Sep 22, 2019

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3919/

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 22, 2019
Trott pushed a commit that referenced this pull request Sep 22, 2019
@ronag @Trott
doc: explain stream.finished cleanup
695db97 
PR-URL: #28935
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@Trott
Copy link
Member

Trott commented Sep 22, 2019 •
edited
Loading

Landed in 4c40a64

@Trott Trott closed this Sep 22, 2019
Trott pushed a commit that referenced this pull request Sep 22, 2019
@ronag @Trott
doc: explain stream.finished cleanup
4c40a64 
PR-URL: #28935
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Sep 23, 2019
@ronag @targos
doc: explain stream.finished cleanup
118b8bf 
PR-URL: #28935
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Sep 24, 2019
Merged
BridgeAR pushed a commit that referenced this pull request Sep 25, 2019
@ronag @BridgeAR
doc: explain stream.finished cleanup
3ba6464 
PR-URL: #28935
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This was referenced Sep 25, 2019
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina left review comments

@Trott Trott Trott approved these changes

@lpinca lpinca lpinca approved these changes

+1 more reviewer

@vsemozhetbyt vsemozhetbyt vsemozhetbyt left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, 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.

6 participants

@ronag @vsemozhetbyt @Trott @mcollina @lpinca @nodejs-github-bot