Skip to content
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.

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

Address some minor PR feedback #187

Merged
merged 1 commit into from
Mar 23, 2017

Conversation

jasongin
Copy link
Member

Addressing several comments in the PR:

  • Simplify conversion to/from napi_value
  • Better names for CallbackWrapper template params
  • Fatal error when trying to set setter return value
  • Use node::arraysize
  • Replace v8::Handle with v8::Local
  • Align the star after v8::Isolate

Also:

  • Remove some non-ascii whitespace chars that were
    causing some IDEs to auto-add a UTF-8 BOM

@jasongin
Copy link
Member Author

jasongin commented Mar 22, 2017

digitalinfinity

This comment was marked as off-topic.

mhdawson

This comment was marked as off-topic.

@jasongin
Copy link
Member Author

@mhdawson What question about whitespace changes? Did you see the note about whitespace in the PR description?

boingoing

This comment was marked as off-topic.

@jasongin
Copy link
Member Author

jasongin commented Mar 22, 2017

gabrielschulhof

This comment was marked as off-topic.

 - Simplify conversion to/from napi_value
 - Better names for CallbackWrapper template params
 - Fatal error when trying to set setter return value
 - Use node::arraysize
 - Replace v8::Handle with v8::Local
 - Align the star after v8::Isolate
@jasongin
Copy link
Member Author

@jasongin jasongin merged commit 65ac2f3 into nodejs:api-prototype-8.x Mar 23, 2017
@jasongin jasongin deleted the feedback1 branch March 23, 2017 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants