Skip to content

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Apr 10, 2017

Missed in ca786c3. This does
not actually affect the outcome because returning nullptr or
this from a constructor has the same effect.

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, n-api

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 10, 2017
@addaleax addaleax added the node-api Issues and PRs related to the Node-API. label Apr 10, 2017
Missed in ca786c3. This does
not actually affect the outcome because returning `nullptr` or
`this` from a constructor has the same effect.
@addaleax addaleax force-pushed the napi-test-constructor-warning branch from 1a1c90b to b4bedf5 Compare April 10, 2017 22:13
@addaleax
Copy link
Member Author

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

This should also be trivial enough not to wait 48 hours.

@mhdawson
Copy link
Member

You beat me to it, I saw this while building another test and was going to look at it today :)

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@addaleax
Copy link
Member Author

Landed in 8bd26d3

@addaleax addaleax closed this Apr 13, 2017
@addaleax addaleax deleted the napi-test-constructor-warning branch April 13, 2017 21:04
addaleax added a commit that referenced this pull request Apr 13, 2017
Missed in ca786c3. This does
not actually affect the outcome because returning `nullptr` or
`this` from a constructor has the same effect.

PR-URL: #12318
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@jasnell jasnell mentioned this pull request May 11, 2017
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 10, 2018
Missed in ca786c3. This does
not actually affect the outcome because returning `nullptr` or
`this` from a constructor has the same effect.

PR-URL: nodejs#12318
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 16, 2018
Missed in ca786c3. This does
not actually affect the outcome because returning `nullptr` or
`this` from a constructor has the same effect.

Backport-PR-URL: #19447
PR-URL: #12318
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
node-api Issues and PRs related to the Node-API. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants