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

doc: stream.finished cleanup by ronag · Pull Request #28935 · nodejs/node · GitHub

/ node Public

doc: stream.finished cleanup - #28935

Closed
ronag wants to merge 2 commits into
nodejs:masterfrom
nxtedition:doc-finished-cleanup
ConversationCommits (2)ChecksFiles changed
Closed

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

Conversation

ronag commented Aug 2, 2019

Copy link
Copy Markdown
Member

Documents stream.finished cleanup behaviour.

Checklist

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 mentioned this pull request Aug 2, 2019
Closed
4 tasks
ronag changed the title docs: stream.finished cleanup doc: stream.finished cleanup Aug 2, 2019
ronag force-pushed the doc-finished-cleanup branch 3 times, most recently from db172c7 to 12448b1 Compare August 2, 2019 07:04
mcollina reviewed Aug 2, 2019
View reviewed changes
Comment thread doc/api/stream.md Outdated
ronag force-pushed the doc-finished-cleanup branch 2 times, most recently from 7b2d4af to 07f5751 Compare August 2, 2019 07:14
vsemozhetbyt reviewed Aug 2, 2019
View reviewed changes
Comment thread doc/api/stream.md Outdated
vsemozhetbyt reviewed Aug 2, 2019
View reviewed changes
Comment thread doc/api/stream.md Outdated
vsemozhetbyt reviewed Aug 2, 2019
View reviewed changes
Comment thread doc/api/stream.md Outdated
vsemozhetbyt reviewed Aug 2, 2019
View reviewed changes
Comment thread doc/api/stream.md Outdated
vsemozhetbyt reviewed Aug 2, 2019
View reviewed changes
Comment thread doc/api/stream.md Outdated

vsemozhetbyt commented Aug 2, 2019

Copy link
Copy Markdown
Contributor

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

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

Trott Aug 2, 2019

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

Unrelated whitespace change?

mcollina Aug 3, 2019

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

@ronag ^^

Trott approved these changes Aug 2, 2019
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

LGTM assuming nits/suggestions are applied.

ronag force-pushed the doc-finished-cleanup branch 2 times, most recently from afd8eff to 264bb84 Compare August 3, 2019 10:15
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 commented Aug 20, 2019

Copy link
Copy Markdown
Member

Rebase to remove conflict?

Trott commented Aug 20, 2019

Copy link
Copy Markdown
Member

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

Trott added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. and removed author ready PRs that have at least one approval, no outstanding review comments, and a CI started. labels Aug 20, 2019

ronag commented Aug 23, 2019

Copy link
Copy Markdown
Member Author

Fixed travis lint failure.

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

ronag commented Sep 18, 2019

Copy link
Copy Markdown
Member Author

@Trott: Is this missing anything?

Trott commented Sep 22, 2019

Copy link
Copy Markdown
Member

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

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

Trott commented Sep 22, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Landed in 4c40a64

Trott closed this Sep 22, 2019
Trott pushed a commit that referenced this pull request Sep 22, 2019
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
doc: explain stream.finished cleanup
118b8bf 
PR-URL: #28935
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR mentioned this pull request Sep 24, 2019
Merged
BridgeAR pushed a commit that referenced this pull request Sep 25, 2019
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
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 left review comments
Trott Trott approved these changes
lpinca lpinca approved these changes
+1 more reviewer
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 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.

6 participants

Back | FazBrowse Home | New Git URL