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: suggest Buffer.alloc instead of Buffer#fill #10000

Merged
merged 0 commits into from
Dec 3, 2016
Merged

doc: suggest Buffer.alloc instead of Buffer#fill #10000

merged 0 commits into from
Dec 3, 2016

Conversation

not-an-aardvark
Copy link
Contributor

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

doc

Description of change

Now that Buffer.alloc exists, there is no reason to recommend using
new Buffer(size).fill(0) or Buffer.allocUnsafe(size).fill(0).

@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. labels Dec 1, 2016
@imyller imyller added code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. doc Issues and PRs related to the documentations. buffer Issues and PRs related to the buffer subsystem. and removed buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. labels Dec 1, 2016
@not-an-aardvark not-an-aardvark removed the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Dec 1, 2016
@imyller
Copy link
Member

imyller commented Dec 1, 2016

Congratulations for PR #10000, @not-an-aardvark !

@imyller
Copy link
Member

imyller commented Dec 1, 2016

And there is also matching commit hash to commemorate the event 👍

Trott
Trott approved these changes Dec 2, 2016
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM

jasnell
jasnell approved these changes Dec 2, 2016
View reviewed changes
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM with a nit

doc/api/buffer.md
@@ -398,7 +398,7 @@ A zero-length `Buffer` will be created if `size <= 0`.

Unlike [`ArrayBuffers`][`ArrayBuffer`], the underlying memory for `Buffer` instances
created in this way is *not initialized*. The contents of a newly created `Buffer`
are unknown and *could contain sensitive data*. Use [`buf.fill(0)`][`buf.fill()`]
are unknown and *could contain sensitive data*. Use [`Buffer.alloc(size)`][`Buffer.alloc()`] instead
Copy link
Member

Choose a reason for hiding this comment

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

nit: long line here

@not-an-aardvark not-an-aardvark merged commit ad6a645 into nodejs:master Dec 3, 2016
@not-an-aardvark not-an-aardvark deleted the recommend-alloc branch December 3, 2016 23:35
@not-an-aardvark
Copy link
Contributor Author

Landed in ad6a645

addaleax pushed a commit that referenced this pull request Dec 5, 2016
@not-an-aardvark @addaleax
doc: suggest Buffer.alloc instead of Buffer#fill
d226418 
Now that `Buffer.alloc` exists, there is no reason to recommend using
`new Buffer(size).fill(0)` or `Buffer.allocUnsafe(size).fill(0)`.

PR-URL: #10000
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Dec 5, 2016
Merged
2 tasks
jmdarling pushed a commit to jmdarling/node that referenced this pull request Dec 8, 2016
@not-an-aardvark
doc: suggest Buffer.alloc instead of Buffer#fill
ca7ca43 
Now that `Buffer.alloc` exists, there is no reason to recommend using
`new Buffer(size).fill(0)` or `Buffer.allocUnsafe(size).fill(0)`.

PR-URL: nodejs#10000
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 20, 2016
@not-an-aardvark @MylesBorins
doc: suggest Buffer.alloc instead of Buffer#fill
adb3067 
Now that `Buffer.alloc` exists, there is no reason to recommend using
`new Buffer(size).fill(0)` or `Buffer.allocUnsafe(size).fill(0)`.

PR-URL: #10000
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
@not-an-aardvark @MylesBorins
doc: suggest Buffer.alloc instead of Buffer#fill
bbffeb9 
Now that `Buffer.alloc` exists, there is no reason to recommend using
`new Buffer(size).fill(0)` or `Buffer.allocUnsafe(size).fill(0)`.

PR-URL: #10000
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 21, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@Trott Trott Trott approved these changes

@targos targos targos approved these changes

@cjihrig cjihrig cjihrig approved these changes

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.

None yet
8 participants
@not-an-aardvark @imyller @jasnell @Trott @targos @cjihrig @MylesBorins @nodejs-github-bot