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

doc: make it more exact by MoonBall · Pull Request #18375 · nodejs/node · GitHub

/ node Public

doc: make it more exact - #18375

Closed
MoonBall wants to merge 3 commits into
nodejs:masterfrom
MoonBall:modify-doc-of-stream
Closed

doc: make it more exact#18375
MoonBall wants to merge 3 commits into
nodejs:masterfrom
MoonBall:modify-doc-of-stream

Conversation

Copy link
Copy Markdown
Member

I made a comment on every change.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

stream

nodejs-github-bot added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. labels Jan 25, 2018
Comment thread doc/api/stream.md Outdated
handlers, and removing all pipe destinations by calling the
[`stream.unpipe()`][] method.
* If there are pipe destinations, by removing all pipe destinations by
calling the [`stream.unpipe()`][] method.

MoonBall Jan 25, 2018
edited
Loading

Copy link
Copy Markdown
Member Author

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

only calling unpipe() will make stream to paused mode. see the following example.

const { Readable, Writable } = require('stream');

const r = new Readable();
const w = new Writable();

r._read = () => {};
w._write = () => {};

r.pipe(w);
r.on('data', () => {});

r.unpipe();
console.log(r._readableState.flowing); // the output is false

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

quick grammar nit on this: by removing all pipe destinations by ... the sentence should be something like:

Multiple pipe destinations may be removed by calling the [`stream.unpipe()`][] method.

Copy link
Copy Markdown
Member Author

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

fixed.

Comment thread doc/api/stream.md Outdated
buffered. Once the `callback` is invoked, the stream will emit a [`'drain'`][]
event. If a stream implementation is capable of processing multiple chunks of
data at once, the `writable._writev()` method should be implemented.
buffered. When the `callback` is invoked, the stream maybe will emit a

Copy link
Copy Markdown
Member Author

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

Emitting a 'drain' event need to meet more conditions. See the source code of writable stream.

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

maybe will -> might

Copy link
Copy Markdown
Member Author

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

fixed.

Copy link
Copy Markdown
Contributor

BridgeAR added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jan 31, 2018
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Feb 1, 2018
PR-URL: nodejs#18375
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

BridgeAR commented Feb 1, 2018

Copy link
Copy Markdown
Member

Landed in 3ca7935

BridgeAR closed this Feb 1, 2018
addaleax removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Feb 4, 2018
MylesBorins pushed a commit that referenced this pull request Feb 20, 2018
PR-URL: #18375
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
PR-URL: #18375
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
PR-URL: #18375
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins mentioned this pull request Feb 21, 2018
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
PR-URL: #18375
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Mar 28, 2018
PR-URL: #18375
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Mar 30, 2018
PR-URL: #18375
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins mentioned this pull request May 2, 2018
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
PR-URL: nodejs#18375
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants


Back | FazBrowse Home | New Git URL