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.

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

doc: Updated streams simplified constructor API #3602

Closed
wants to merge 1 commit into from

Conversation

tomgco
Copy link
Contributor

@tomgco tomgco commented Oct 30, 2015

The examples for implementing the simplified constructor API was missing some details on its correct usages.

I have updated them to reflect this.

@Fishrock123 Fishrock123 added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. labels Oct 30, 2015
@@ -1384,6 +1388,9 @@ var readable = new stream.Readable({
var writable = new stream.Writable({
write: function(chunk, encoding, next) {
// sets this._write under the hood

// Optionally with an error
Copy link
Contributor

Choose a reason for hiding this comment

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

you mean an error can be optionally passed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that would sound more concise, will update.

@tomgco
Copy link
Contributor Author

tomgco commented Nov 6, 2015

@trevnorris I have addressed your comments and updated the PR

@trevnorris
Copy link
Contributor

Thanks much.

Anyone from @nodejs/documentation or such mind weighing in?

@chrisdickinson
Copy link
Contributor

LGTM!

On Nov 6, 2015, at 7:46 AM, Trevor Norris notifications@github.com wrote:

Thanks much.

Anyone from @nodejs/documentation or such mind weighing in?


Reply to this email directly or view it on GitHub.

@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

LGTM. There are some merge conflicts due to the doc being restructured. I'll fix those up on landing tho.

jasnell pushed a commit that referenced this pull request Nov 13, 2015
The examples for implementing the simplified constructor API
was missing some details on its correct usages.

PR-URL: #3602
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

Landed in 4270e7f

@jasnell jasnell closed this Nov 13, 2015
MylesBorins pushed a commit that referenced this pull request Nov 16, 2015
The examples for implementing the simplified constructor API
was missing some details on its correct usages.

PR-URL: #3602
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
@MylesBorins
Copy link
Contributor

landed in v4.x-staging as bb20abb

Fishrock123 pushed a commit that referenced this pull request Nov 17, 2015
The examples for implementing the simplified constructor API
was missing some details on its correct usages.

PR-URL: #3602
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
rvagg pushed a commit that referenced this pull request Dec 4, 2015
The examples for implementing the simplified constructor API
was missing some details on its correct usages.

PR-URL: #3602
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
@jasnell jasnell mentioned this pull request Dec 17, 2015
jasnell pushed a commit that referenced this pull request Dec 17, 2015
The examples for implementing the simplified constructor API
was missing some details on its correct usages.

PR-URL: #3602
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
jasnell pushed a commit that referenced this pull request Dec 23, 2015
The examples for implementing the simplified constructor API
was missing some details on its correct usages.

PR-URL: #3602
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
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.

6 participants