Skip to content

doc,test: args to buffer.copy can be Uint8Arrays #11486

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
Closed
wants to merge 1 commit into from
Closed

doc,test: args to buffer.copy can be Uint8Arrays #11486

wants to merge 1 commit into from

Conversation

@addaleax
Copy link
Member

@addaleax addaleax commented Feb 21, 2017 •
edited
Loading

This was semi-overlooked in #10236 because it always worked and didn’t need additional changes.

Ref: #10236

Checklist
  • 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
Affected core subsystem(s)

buffer

CI: https://ci.nodejs.org/job/node-test-commit/8059/

@addaleax addaleax added buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. labels Feb 21, 2017
Trott
Trott approved these changes Feb 21, 2017
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 if CI is green

lpinca
lpinca reviewed Feb 22, 2017
View reviewed changes
test/parallel/test-buffer-copy.js Outdated
Copy link
Member

@lpinca lpinca Feb 22, 2017

Choose a reason for hiding this comment

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

s/doo/too/ ?

Copy link
Member Author

@addaleax addaleax Feb 22, 2017

Choose a reason for hiding this comment

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

@lpinca lol yes, thanks for pointing out. done! 😄

@addaleax
doc,test: args to buffer.copy can be Uint8Arrays
6e49735 
This was semi-overlooked in nodejs#10236
because it always worked and didn’t need additional changes.

Ref: nodejs#10236
@addaleax addaleax force-pushed the buf-copy-uint8array branch from 2851c32 to 6e49735 Compare February 22, 2017 17:55
@addaleax
Copy link
Member Author

addaleax commented Feb 24, 2017

Landed in cba1020

@addaleax addaleax closed this Feb 24, 2017
@addaleax addaleax deleted the buf-copy-uint8array branch February 24, 2017 01:00
addaleax added a commit that referenced this pull request Feb 24, 2017
@addaleax
doc,test: args to buffer.copy can be Uint8Arrays
cba1020 
This was semi-overlooked in #10236
because it always worked and didn’t need additional changes.

Ref: #10236
PR-URL: #11486
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax added a commit that referenced this pull request Feb 24, 2017
@addaleax
doc,test: args to buffer.copy can be Uint8Arrays
89b66dc 
This was semi-overlooked in #10236
because it always worked and didn’t need additional changes.

Ref: #10236
PR-URL: #11486
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@italoacasas italoacasas mentioned this pull request Feb 25, 2017
Merged
@jasnell
Copy link
Member

jasnell commented Mar 7, 2017

This will need a backport PR to land in v4.x

jasnell pushed a commit that referenced this pull request Mar 7, 2017
@addaleax @jasnell
doc,test: args to buffer.copy can be Uint8Arrays
cd1dca6 
This was semi-overlooked in #10236
because it always worked and didn’t need additional changes.

Ref: #10236
PR-URL: #11486
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
@addaleax @MylesBorins
doc,test: args to buffer.copy can be Uint8Arrays
b503824 
This was semi-overlooked in #10236
because it always worked and didn’t need additional changes.

Ref: #10236
PR-URL: #11486
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
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

@lpinca lpinca lpinca 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. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@addaleax @jasnell @Trott @lpinca