doc: fix missing Buffer.of method in docs/buffer
#21439
Conversation
Note: Part added needs verification; however is commented out in case it cannot be verified or is otherwise deemed unnecessary for users.
nodejs-github-bot
added
buffer
|
I'm not sure we need to document methods inherited from |
| added: 6.0.0 | ||
| --> | ||
|
|
||
| * `...items` {number[]} A list of values for buffer initialisation |
There was a problem hiding this comment.
{integer}
initialization (we use US spelling)
| added: 6.0.0 | ||
| --> | ||
|
|
||
| * `...items` {number[]} A list of values for buffer initialisation |
| several internal setup calls. | ||
|
|
||
| ```js | ||
| Buffer.from(1, 2, 3, 4).equals(Buffer.from([1, 2, 3, 4])) |
There was a problem hiding this comment.
The first call should be Buffer.of(). End the line with a semicolon. Also, it would be helpful to have the return value of .equals() as a comment.
| several internal setup calls. | ||
|
|
||
| ```js | ||
| Buffer.from(1, 2, 3, 4).equals(Buffer.from([1, 2, 3, 4])) |
There was a problem hiding this comment.
The first call should be Buffer.of(). End the line with a semicolon. Also, it would be helpful to have the return value of .equals() as a comment.
| using the deprecated `Buffer()` constructor. <!--Additionally, | ||
| `Buffer.of` may be more performant as it creates and | ||
| fills the Buffer directly, and does not create a `this` | ||
| reference as it uses arrow syntax. --> |
There was a problem hiding this comment.
No need to mention "avoids using the constructor," an implementation detail.
I'd specifically mention Uint8Array.of rather than the generic %TypedArray%.of.
I'd like to see some evidence that Buffer.of is more efficient than Buffer.from – if that's true then I would uncomment the additional info, and if not or unable to be verified then the comment should be deleted. Either way there should be no comment.
There's also no need to mention this reference or arrow syntax, which is an implementation detail.
|
@mscdex It's no longer inherited from @ZaneHannanAU Please also take a look at the Travis CI failure. |
|
@TimothyGu I meant inherited in any sense. The behavior of our implementation should match that of |
|
What is the status of this PR? Would it proceed? |
|
@vsemozhetbyt unlikely. The main reason for is that the function does not inherit from its parent; the main against is that the function does follow the same rules as its parent. |
|
Thank you. So can we close it for now? We can always reconsider later. |
|
I am fine with closing it; I just want a formal yes/no; at least once I've fixed it now I remember it… anyway; once I fix it up and the build passes if it's not worth the time let's leave it be. For now I'll close it though. |
|
To be on the safe side, cc @nodejs/buffer |
|
I'm going to close this as the branch has apparently been deleted so I can't even update the branch. Re-open or leave a comment if it should be left open. If anyone else wants to document |
Note: Part added needs verification; however
is commented out in case it cannot be verified
or is otherwise deemed unnecessary for users.
Checklist