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

ThomasdenH · 2017-09-20T09:54:05Z

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

lpinca · 2017-09-20T10:04:04Z

doc/api/buffer.md


 [`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


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

ThomasdenH · 2017-09-20T10:12:47Z

Done. I can rebase the commits if you prefer.

lpinca · 2017-09-20T10:14:54Z

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

TimothyGu · 2017-09-20T16:37:37Z

doc/api/buffer.md


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


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

TimothyGu · 2017-09-20T16:37:50Z

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 · 2017-09-20T16:38:18Z

doc/api/buffer.md

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


BridgeAR

LGTM with the comments addressed.

ThomasdenH · 2017-09-24T17:24:59Z

I've addressed the comments.

BridgeAR · 2017-09-24T17:35:37Z

doc/api/buffer.md


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


The line length is above 80 chars.

BridgeAR · 2017-09-24T17:35:57Z

doc/api/buffer.md

 -->

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


The line length is above 80 chars.

BridgeAR · 2017-09-24T17:36:00Z

doc/api/buffer.md


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


The line length is above 80 chars.

BridgeAR · 2017-10-01T22:17:53Z

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

BridgeAR · 2017-11-22T13:41:09Z

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

jasnell · 2017-11-22T17:41:22Z

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

targos · 2017-11-23T13:49:31Z

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 · 2017-11-23T14:40:57Z

Landed in 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>

add SharedArrayBuffer to Buffer documentation

abdbe64

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

alphabetize hyperlinks

9fd40da

joyeecheung approved these changes Sep 20, 2017

View reviewed changes

cjihrig approved these changes Sep 20, 2017

View reviewed changes

TimothyGu reviewed Sep 20, 2017

View reviewed changes

jasnell approved these changes Sep 20, 2017

View reviewed changes

BridgeAR approved these changes Sep 24, 2017

View reviewed changes

Update JSDoc types

9d023d5

BridgeAR reviewed Sep 24, 2017

View reviewed changes

ThomasdenH added 2 commits October 4, 2017 12:13

Wrap long lines

beb2524

Merge branch 'master' into patch-2

0924c71

BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 22, 2017

targos approved these changes Nov 23, 2017

View reviewed changes

targos closed this Nov 23, 2017

joyeecheung mentioned this pull request Nov 23, 2017

pr_checker: ignore private emails nodejs/node-core-utils#121

Merged

addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 28, 2017

MylesBorins mentioned this pull request Dec 12, 2017

v9.3.0 proposal #17631

Merged

gibfahn added the land-on-v8.x label Dec 19, 2017

gibfahn added the land-on-v6.x label Dec 19, 2017

gibfahn mentioned this pull request Dec 20, 2017

v8.9.4 proposal #17772

Closed

gibfahn mentioned this pull request Dec 20, 2017

v8.9.4 proposal #17774

Merged

MylesBorins mentioned this pull request Dec 20, 2017

v6.12.3 proposal #17776

Merged

Uh oh!

Conversation

ThomasdenH commented Sep 20, 2017

Uh oh!

lpinca Sep 20, 2017

Choose a reason for hiding this comment

Uh oh!

ThomasdenH commented Sep 20, 2017

Uh oh!

lpinca commented Sep 20, 2017

Uh oh!

TimothyGu Sep 20, 2017

Choose a reason for hiding this comment

Uh oh!

TimothyGu Sep 20, 2017

Choose a reason for hiding this comment

Uh oh!

TimothyGu Sep 20, 2017

Choose a reason for hiding this comment

Uh oh!

BridgeAR left a comment

Choose a reason for hiding this comment

Uh oh!

ThomasdenH commented Sep 24, 2017

Uh oh!

BridgeAR Sep 24, 2017

Choose a reason for hiding this comment

Uh oh!

BridgeAR Sep 24, 2017

Choose a reason for hiding this comment

Uh oh!

BridgeAR Sep 24, 2017

Choose a reason for hiding this comment

Uh oh!

BridgeAR commented Oct 1, 2017

Uh oh!

BridgeAR commented Nov 22, 2017

Uh oh!

jasnell commented Nov 22, 2017

Uh oh!

targos commented Nov 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

targos commented Nov 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

targos commented Nov 23, 2017 •

edited

Loading