Skip to content
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

doc,stream: remove wrong remark on readable.read #15014

Closed
wants to merge 1 commit into from
Closed

doc,stream: remove wrong remark on readable.read #15014

wants to merge 1 commit into from

Conversation

jscissr
Copy link
Contributor

@jscissr jscissr commented Aug 24, 2017

The returned chunk is never longer than size, if I read the code correctly. I even tested it:

const stream = require('stream')

const readable = new stream.Readable({
  read (size) {
    this.push(Buffer.from('0123456789'))
    this.push(null)
  }
})

readable.on('readable', () => {
  let buf
  while ((buf = readable.read(4))) {
    console.log(buf)
  }
})

Output:

<Buffer 30 31 32 33>
<Buffer 34 35 36 37>
<Buffer 38 39>

Even tough the stream has ended, the data returned does not exceed size bytes.

Checklist
Affected core subsystem(s)

stream

@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 24, 2017
jasnell
jasnell reviewed Aug 24, 2017
View reviewed changes
test/async-hooks/test-graph.tcp.js Outdated
@@ -17,7 +17,7 @@ const server = net

server.listen(common.PORT);

net.connect({ port: server.address().port, host: server.address().address },
net.connect({ port: server.address().port, host: '::1' },
Copy link
Member

Choose a reason for hiding this comment

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

unrelated change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know from where this commit appeared.It might be related to the "allow edits from maintainers" option on the pull request. Or maybe this commit was deleted from master, but is still in my branch?
It also affects #15013

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems to be the reason. I will rebase on master.

@jasnell jasnell requested a review from mcollina August 24, 2017 15:34
mcollina
mcollina approved these changes Aug 24, 2017
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM without the spurious changes.

@jscissr
doc,stream: remove wrong remark on readable.read
a560ae8 
The returned chunk is *never* longer than `size`.
@jscissr
Copy link
Contributor Author

jscissr commented Aug 24, 2017

Did a rebase now.

jasnell pushed a commit that referenced this pull request Aug 25, 2017
@jscissr @jasnell
doc,stream: remove wrong remark on readable.read
8987ae8 
The returned chunk is *never* longer than `size`.

PR-URL: #15014
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@jasnell
Copy link
Member

jasnell commented Aug 25, 2017

Landed in 8987ae8

@jasnell jasnell closed this Aug 25, 2017
@jscissr jscissr deleted the patch-1 branch August 30, 2017 13:53
ghost pushed a commit to ayojs/ayo that referenced this pull request Aug 30, 2017
@jscissr
doc,stream: remove wrong remark on readable.read
b93fc5b 
The returned chunk is *never* longer than `size`.

PR-URL: nodejs/node#15014
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
ghost pushed a commit to ayojs/ayo that referenced this pull request Aug 30, 2017
@jscissr
doc,stream: remove wrong remark on readable.read
4d6150b 
The returned chunk is *never* longer than `size`.

PR-URL: nodejs/node#15014
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 10, 2017
@jscissr @MylesBorins
doc,stream: remove wrong remark on readable.read
2dbaa74 
The returned chunk is *never* longer than `size`.

PR-URL: #15014
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Sep 10, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
@jscissr @MylesBorins
doc,stream: remove wrong remark on readable.read
7f62378 
The returned chunk is *never* longer than `size`.

PR-URL: #15014
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 20, 2017
@jscissr @MylesBorins
doc,stream: remove wrong remark on readable.read
f0328f6 
The returned chunk is *never* longer than `size`.

PR-URL: #15014
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Sep 20, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

Assignees
No one assigned
Labels
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
@jscissr @jasnell @mcollina @lpinca @MylesBorins @nodejs-github-bot