Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

doc: apply multiple doc updates from master to v0.12 #25591

Closed
wants to merge 20 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jun 29, 2015

Applies the several doc updates made to master to v0.12.

jasnell and others added 7 commits June 29, 2015 09:38
Per nodejs#4409, the documentation on http.abort is a bit lacking.
This provides a slight improvement.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25565
The documentation for createWriteStream() references an
'encoding' property that has a default value of null. However,
this property is never referenced by createWriteStream() or
WritableState(). Instead a 'defaultEncoding' property is
referenced in WritableState() with a default of 'utf8' if no value
is supplied.

This fix updates the documentation to rename the 'encoding'
property to 'defaultEncoding' and indicate its default value of
'utf8'.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25565
Fix the line wrapping in buffer.markdown

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25565
Original: nodejs#8682

Slightly modified version of the original PR (nodejs#8682) to add
appropriate line wrapping and fix a couple of grammar nits.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25565
@jasnell
Copy link
Member Author

jasnell commented Jul 9, 2015

@joyent/node-tsc ... can I get a quick review on this? These previously already landed in master, just backporting them to v0.12

scop and others added 5 commits July 9, 2015 16:44
'the' to 'then'

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25592
Made explicitely clear that when size bytes are not available, it will
return null, unless we've ended, in which case the data remaining in the
buffer will be returned.

Fixes nodejs#7273

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25592
@jasnell
Copy link
Member Author

jasnell commented Jul 9, 2015

Added a few additional doc merges that had landed in master

bsteephenson and others added 7 commits July 10, 2015 10:34
Clarifies that emitter.listener() returns a copy, not a reference
Resolves issue nodejs#9022

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25635
per: nodejs#14596

1. document that a runtime error will occur if you attempt
   to unshift after the end event
2. document that calling read after the end event will return
   null and will not trigger a runtime error

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25635
Minor clarifications around Readable._read and Readable.push
to make their implementation/usage easier to understand.

nodejs#14124 (comment)

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25635
Fix for nodejs#25559 (Typo in example of util.deprecate() documentation)

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25635
Per nodejs#14604,

Document that performing an `unshift` during a read
can have unexpected results. Following the `unshift`
with a `push('')` resets the reading state appropriately.
Also indicate that doing an `unshift` during a read
is not optimal and should be avoided.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25635
per nodejs#14597

Indicate that `'readable'` indicates only that data can
be read from the stream, not that there is actually data
to be consumed. `readable.read([size])` can still return
null. Includes an example that illustrates the point.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25635
Per nodejs#25635 (comment)

Additional refinement to the clarification on the `readable` event

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25635
@jasnell
Copy link
Member Author

jasnell commented Jul 10, 2015

Added a few more. These have all been reviewed and landed in master.

@@ -181,6 +209,9 @@ readable.on('data', function(chunk) {
console.log('got %d bytes of data', chunk.length);
});
```
Note that the `readable` event should not be used together with `data`
because the assigning the latter switches the stream into "flowing" mode,

Choose a reason for hiding this comment

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

@jasnell "the assigning the latter" should probably be reworded. Maybe just "because listening on the latter"?

@misterdjules
Copy link

@jasnell Added a few comments.

In general, I'm not sure that the method of landing several commits in master to have them back ported in batch via a PR is a good workflow. It seems that it adds to the manual steps needed to land changes. More reviews of bigger changes are needed, and it also seems like it would be made worse if we needed to back port some of these changes to v0.10.

It seems that landing these changes one by one in the oldest supported branch where they apply and then letting them bubble up along with the merges from "older" branches up to master and via v0.12 doesn't have these drawbacks.

Did you experience any benefit in doing it by back porting?

@jasnell
Copy link
Member Author

jasnell commented Aug 4, 2015

In general, yes, doing it this was ends up being a significant time saver. These were all already previously reviewed and signed off as PR's against master.

jasnell added a commit to jasnell/node that referenced this pull request Aug 4, 2015
Original: nodejs/node-v0.x-archive#8682

Slightly modified version of the original PR (nodejs#8682) to add
appropriate line wrapping and fix a couple of grammar nits.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node-v0.x-archive#25591
jasnell pushed a commit to jasnell/node that referenced this pull request Aug 4, 2015
jasnell pushed a commit to jasnell/node that referenced this pull request Aug 4, 2015
Made explicitely clear that when size bytes are not available,
it will return null, unless we've ended, in which case the data
remaining in the buffer will be returned.

Fixes nodejs#7273

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node-v0.x-archive#25591
jasnell pushed a commit to jasnell/node that referenced this pull request Aug 4, 2015
Clarifies that emitter.listener() returns a copy, not a reference
Resolves issue nodejs#9022

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node-v0.x-archive#25591
jasnell added a commit to jasnell/node that referenced this pull request Aug 4, 2015
per: nodejs/node-v0.x-archive#14596

1. document that a runtime error will occur if you attempt
   to unshift after the end event
2. document that calling read after the end event will return
   null and will not trigger a runtime error

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node-v0.x-archive#25591
jasnell pushed a commit to jasnell/node that referenced this pull request Aug 4, 2015
Minor clarifications around Readable._read and Readable.push
to make their implementation/usage easier to understand.

nodejs/node-v0.x-archive#14124 (comment)

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node-v0.x-archive#25591
jasnell added a commit to jasnell/node that referenced this pull request Aug 4, 2015
Per nodejs/node-v0.x-archive#14604,

Document that performing an `unshift` during a read
can have unexpected results. Following the `unshift`
with a `push('')` resets the reading state appropriately.
Also indicate that doing an `unshift` during a read
is not optimal and should be avoided.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node-v0.x-archive#25591
jasnell added a commit to jasnell/node that referenced this pull request Aug 4, 2015
per nodejs/node-v0.x-archive#14597

Indicate that `'readable'` indicates only that data can
be read from the stream, not that there is actually data
to be consumed. `readable.read([size])` can still return
null. Includes an example that illustrates the point.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node-v0.x-archive#25591
jasnell added a commit to jasnell/node that referenced this pull request Aug 4, 2015
Per nodejs/node-v0.x-archive#25635 (comment)

Additional refinement to the clarification on the `readable` event

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node-v0.x-archive#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Per nodejs#4409, the documentation on http.abort is a bit lacking.
This provides a slight improvement.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
The documentation for createWriteStream() references an
'encoding' property that has a default value of null. However,
this property is never referenced by createWriteStream() or
WritableState(). Instead a 'defaultEncoding' property is
referenced in WritableState() with a default of 'utf8' if no value
is supplied.

This fix updates the documentation to rename the 'encoding'
property to 'defaultEncoding' and indicate its default value of
'utf8'.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Fix the line wrapping in buffer.markdown

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Original: nodejs#8682

Slightly modified version of the original PR (nodejs#8682) to add
appropriate line wrapping and fix a couple of grammar nits.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
'the' to 'then'

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Made explicitely clear that when size bytes are not available, it will
return null, unless we've ended, in which case the data remaining in the
buffer will be returned.

Fixes nodejs#7273

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Clarifies that emitter.listener() returns a copy, not a reference
Resolves issue nodejs#9022

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
per: nodejs#14596

1. document that a runtime error will occur if you attempt
   to unshift after the end event
2. document that calling read after the end event will return
   null and will not trigger a runtime error

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Minor clarifications around Readable._read and Readable.push
to make their implementation/usage easier to understand.

nodejs#14124 (comment)

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Fix for nodejs#25559 (Typo in example of util.deprecate() documentation)

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Per nodejs#14604,

Document that performing an `unshift` during a read
can have unexpected results. Following the `unshift`
with a `push('')` resets the reading state appropriately.
Also indicate that doing an `unshift` during a read
is not optimal and should be avoided.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
per nodejs#14597

Indicate that `'readable'` indicates only that data can
be read from the stream, not that there is actually data
to be consumed. `readable.read([size])` can still return
null. Includes an example that illustrates the point.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Per nodejs#25635 (comment)

Additional refinement to the clarification on the `readable` event

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#25591
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.