Skip to content

Conversation

@DavidCai1111
Copy link
Member

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Feb 2, 2017
@hiroppyhiroppy added the buffer Issues and PRs related to the buffer subsystem. label Feb 2, 2017
Copy link
Member

@joyeecheungjoyeecheungFeb 2, 2017

Choose a reason for hiding this comment

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

Something like

consterror=/Int/.test(funx) ? /^TypeError:"buffer"argumentmustbeaBufferorUint8Array$/ : /^TypeError:argumentshouldbeaBuffer$/;assert.throws(()=>Buffer.alloc(9)[funx].apply(newUint32Array(1),args),error);

would probably be easier to read.

Copy link
Member

@jasnelljasnell 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 @joyeecheung's formatting suggestion

@DavidCai1111
Copy link
MemberAuthor

OK, updated.

@italoacasas
Copy link

jasnell pushed a commit that referenced this pull request Feb 6, 2017
PR-URL: #11122 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
@jasnell
Copy link
Member

Landed in 901cb8c

@jasnelljasnell closed this Feb 6, 2017
@italoacasas
Copy link

This is failing in v7.x, maybe because of some semver-major in buffer. Any plan to backport this?

@joyeecheung
Copy link
Member

joyeecheung commented Feb 9, 2017

This depends on semver-major #10236 which adds support for Uint8Array input to buffer methods. Should be manually backportable though. @DavidCai1993 mind submitting a backport for this?

@DavidCai1111
Copy link
MemberAuthor

OK

italoacasas pushed a commit that referenced this pull request Feb 16, 2017
PR-URL: #11312 Backport-of: #11122 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Italo A. Casas <[email protected]>
italoacasas pushed a commit that referenced this pull request Feb 22, 2017
PR-URL: #11312 Backport-of: #11122 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Italo A. Casas <[email protected]>
krydos pushed a commit to krydos/node that referenced this pull request Feb 25, 2017
PR-URL: nodejs#11122 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
@jasnelljasnell mentioned this pull request Apr 4, 2017
@gibfahngibfahn mentioned this pull request Jun 15, 2017
3 tasks
@gibfahn
Copy link
Member

Adding dont-land-on-v6.x due to #11312 (comment), @DavidCai1993 if you think this should be backported please raise a PR (see the backporting guide for the howto.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bufferIssues and PRs related to the buffer subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@DavidCai1111@italoacasas@jasnell@joyeecheung@gibfahn@thefourtheye@hiroppy@nodejs-github-bot@davidtaikocha