FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add SharedArrayBuffer to Buffer documentation by ThomasdenH · Pull Request #15489 · nodejs/node · GitHub

/ node Public

Add SharedArrayBuffer to Buffer documentation - #15489

Closed
ThomasdenH wants to merge 5 commits into
nodejs:masterfrom
ThomasdenH:patch-2
ConversationCommits (5)ChecksFiles changed
Closed

Add SharedArrayBuffer to Buffer documentation#15489
ThomasdenH wants to merge 5 commits into
nodejs:masterfrom
ThomasdenH:patch-2

Conversation

ThomasdenH commented Sep 20, 2017

Copy link
Copy Markdown
Contributor

Since 2a2ec9d, Buffer accepts a SharedArrayBuffer in places where ArrayBuffer was accepted previously. This PR updates the documentation accordingly.

Note that not every occurrence was replaced. In most cases it seems clear from the context that both are accepted, although some might disagree about this.

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. labels Sep 20, 2017
lpinca reviewed Sep 20, 2017
View reviewed changes
Comment thread doc/api/buffer.md Outdated

[`ArrayBuffer#slice()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer/slice
[`ArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer
[`SharedArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer

lpinca Sep 20, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you please move this line to keep the alphabetical order?

ThomasdenH commented Sep 20, 2017

Copy link
Copy Markdown
Contributor Author

Done. I can rebase the commits if you prefer.

lpinca commented Sep 20, 2017

Copy link
Copy Markdown
Member

No, it's fine, commits are squashed before landing. Thank you.

TimothyGu reviewed Sep 20, 2017
View reviewed changes
Comment thread doc/api/buffer.md Outdated

* `arrayBuffer` {ArrayBuffer} An [`ArrayBuffer`] or the `.buffer` property of a
[`TypedArray`].
* `arrayBuffer` {ArrayBuffer} An [`ArrayBuffer`], [`SharedArrayBuffer`] or the

TimothyGu Sep 20, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you change {ArrayBuffer} to {ArrayBuffer|SharedArrayBuffer} as well?

Comment thread doc/api/buffer.md
When `string` is a `Buffer`/[`DataView`]/[`TypedArray`]/[`ArrayBuffer`], the
actual byte length is returned.
When `string` is a `Buffer`/[`DataView`]/[`TypedArray`]/[`ArrayBuffer`]/
[`SharedArrayBuffer`], the actual byte length is returned.

TimothyGu Sep 20, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ditto.

Comment thread doc/api/buffer.md Outdated
* `arrayBuffer` {ArrayBuffer} An [`ArrayBuffer`] or the `.buffer` property of a
[`TypedArray`].
* `arrayBuffer` {ArrayBuffer} An [`ArrayBuffer`], [`SharedArrayBuffer`], or the
`.buffer` property of a [`TypedArray`].

TimothyGu Sep 20, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ditto.

BridgeAR approved these changes Sep 24, 2017
View reviewed changes

BridgeAR left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM with the comments addressed.

ThomasdenH commented Sep 24, 2017

Copy link
Copy Markdown
Contributor Author

I've addressed the comments.

BridgeAR reviewed Sep 24, 2017
View reviewed changes
Comment thread doc/api/buffer.md Outdated

* `arrayBuffer` {ArrayBuffer} An [`ArrayBuffer`] or the `.buffer` property of a
[`TypedArray`].
* `arrayBuffer` {ArrayBuffer|SharedArrayBuffer} An [`ArrayBuffer`], [`SharedArrayBuffer`], or the

BridgeAR Sep 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The line length is above 80 chars.

BridgeAR reviewed Sep 24, 2017
View reviewed changes
Comment thread doc/api/buffer.md Outdated
-->

* `string` {string|Buffer|TypedArray|DataView|ArrayBuffer} A value to
* `string` {string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer} A value to

BridgeAR Sep 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The line length is above 80 chars.

BridgeAR reviewed Sep 24, 2017
View reviewed changes
Comment thread doc/api/buffer.md Outdated

* `arrayBuffer` {ArrayBuffer} An [`ArrayBuffer`] or the `.buffer` property of a
[`TypedArray`].
* `arrayBuffer` {ArrayBuffer|SharedArrayBuffer} An [`ArrayBuffer`], [`SharedArrayBuffer`] or the

BridgeAR Sep 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The line length is above 80 chars.

BridgeAR commented Oct 1, 2017

Copy link
Copy Markdown
Member

Ping @ThomasdenH this needs a rebase and please address the comments about the line length.

BridgeAR added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Nov 22, 2017

BridgeAR commented Nov 22, 2017

Copy link
Copy Markdown
Member

CI https://ci.nodejs.org/job/node-test-pull-request/11629/

jasnell commented Nov 22, 2017

Copy link
Copy Markdown
Member

@ThomasdenH ... can I ask you to please squash the commits down into a single commit

targos commented Nov 23, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Linter CI after rebase and squash: https://ci.nodejs.org/job/node-test-linter/13860/

Edit: CI timed out. New try: https://ci.nodejs.org/job/node-test-linter/13861/

targos commented Nov 23, 2017

Copy link
Copy Markdown
Member

Landed in bd0e36d

targos closed this Nov 23, 2017
targos pushed a commit that referenced this pull request Nov 23, 2017
doc: add SharedArrayBuffer to Buffer documentation
bd0e36d 
PR-URL: #15489
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
joyeecheung mentioned this pull request Nov 23, 2017
Merged
addaleax removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Nov 28, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
doc: add SharedArrayBuffer to Buffer documentation
118a3e2 
PR-URL: #15489
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
doc: add SharedArrayBuffer to Buffer documentation
df5436c 
PR-URL: #15489
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins mentioned this pull request Dec 12, 2017
Merged
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
doc: add SharedArrayBuffer to Buffer documentation
cdb7d24 
PR-URL: #15489
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
doc: add SharedArrayBuffer to Buffer documentation
a0bd1c0 
PR-URL: #15489
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
gibfahn mentioned this pull request Dec 20, 2017
Closed
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
doc: add SharedArrayBuffer to Buffer documentation
36f5f13 
PR-URL: #15489
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
gibfahn mentioned this pull request Dec 20, 2017
Merged
MylesBorins mentioned this pull request Dec 20, 2017
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lpinca lpinca left review comments
BridgeAR BridgeAR approved these changes
jasnell jasnell approved these changes
cjihrig cjihrig approved these changes
joyeecheung joyeecheung approved these changes
+2 more reviewers
TimothyGu TimothyGu left review comments
targos targos approved these changes
Reviewers whose approvals may not affect merge requirements

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.

Back | FazBrowse Home | New Git URL