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: improvements to buffer.markdown copy #4370

Closed
wants to merge 1 commit into from
Closed

doc: improvements to buffer.markdown copy #4370

wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Dec 20, 2015

General improvements to buffer.markdown including new examples,
a few fixes to existing examples, consistent formatting and
others

@jasnell jasnell added buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. labels Dec 20, 2015
@ChALkeR
Copy link
Member

ChALkeR commented Dec 20, 2015

A question from #4341 — is new Buffer(arrayBuffer) not documented for some reason or just missing?

@jasnell
Copy link
Member Author

jasnell commented Dec 21, 2015

Appears to just be missing. I can add it to this.
On Dec 20, 2015 3:55 PM, "Сковорода Никита Андреевич" <
notifications@github.com> wrote:

A question from #4341 #4341 — is new
Buffer(arrayBuffer) not documented for some reason or just missing?

—
Reply to this email directly or view it on GitHub
#4370 (comment).

@jasnell
Copy link
Member Author

jasnell commented Dec 21, 2015

@ChALkeR ... example added!

@jasnell
Copy link
Member Author

jasnell commented Dec 21, 2015

@trevnorris ... would appreciate a review on this. Thanks!

@trevnorris
Copy link
Contributor

Finished first round of reviews. Great changes in here.

@jasnell
Copy link
Member Author

jasnell commented Dec 22, 2015

Commits squashed ...

@cjihrig
Copy link
Contributor

cjihrig commented Dec 22, 2015

Heh after squashing, the diff isn't very conducive to review on GitHub.

@jasnell
Copy link
Member Author

jasnell commented Dec 22, 2015

Ugh. Silly github. Yeah there are quite a few changes there.
On Dec 22, 2015 6:55 AM, "Colin Ihrig" notifications@github.com wrote:

Heh after squashing, the diff isn't very conducive to review on GitHub.

—
Reply to this email directly or view it on GitHub
#4370 (comment).

@trevnorris
Copy link
Contributor

Since I can't do it inline:

size must be less than 1,073,741,824 bytes (1 GB) on 32-bit architectures or 2,147,483,648 bytes (2 GB) on 64-bit architectures.

This is incorrect. A user can check max size by require('buffer').kMaxLength. On x64 it's actually
(2^31)-1 bytes.

Other than that, LGTM.

@jasnell
Copy link
Member Author

jasnell commented Dec 23, 2015

@trevnorris ... ok, addressed that nit.

@trevnorris
Copy link
Contributor

LGTM

@jasnell
doc: improvements to buffer.markdown copy
8f8bc57 
General improvements to buffer.markdown including new examples,
a few fixes to existing examples, consistent formatting and
others
jasnell added a commit that referenced this pull request Dec 23, 2015
@jasnell
doc: improvements to buffer.markdown copy
67b109a 
General improvements to buffer.markdown including new examples,
a few fixes to existing examples, consistent formatting and
others

PR-URL: #4370
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented Dec 23, 2015

Landed in 67b109a

@jasnell jasnell closed this Dec 23, 2015
@MylesBorins
Copy link
Contributor

@jasnell this is not merging cleanly at all on lts.. can you handle this one?

@shigeki shigeki mentioned this pull request Jan 5, 2016
Closed
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Jan 6, 2016
@jasnell @Fishrock123
doc: improvements to buffer.markdown copy
474a0f0 
General improvements to buffer.markdown including new examples,
a few fixes to existing examples, consistent formatting and
others

PR-URL: nodejs#4370
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 28, 2016
Closed
@MylesBorins
Copy link
Contributor

ping @jasnell

@MylesBorins
Copy link
Contributor

when this lands in LTS it must come with #4537

jasnell added a commit to jasnell/node that referenced this pull request Feb 19, 2016
@jasnell
doc: improvements to buffer.markdown copy
ff66f1f 
General improvements to buffer.markdown including new examples,
a few fixes to existing examples, consistent formatting and
others

PR-URL: nodejs#4370
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 22, 2016
@jasnell
doc: improvements to buffer.markdown copy
f6010ee 
General improvements to buffer.markdown including new examples,
a few fixes to existing examples, consistent formatting and
others

PR-URL: #4370
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 22, 2016
@jasnell
doc: improvements to buffer.markdown copy
a929837 
General improvements to buffer.markdown including new examples,
a few fixes to existing examples, consistent formatting and
others

PR-URL: #4370
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
@jasnell
doc: improvements to buffer.markdown copy
819a83b 
General improvements to buffer.markdown including new examples,
a few fixes to existing examples, consistent formatting and
others

PR-URL: #4370
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
@jasnell
doc: improvements to buffer.markdown copy
a450989 
General improvements to buffer.markdown including new examples,
a few fixes to existing examples, consistent formatting and
others

PR-URL: nodejs#4370
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jasnell @ChALkeR @trevnorris @cjihrig @MylesBorins