Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

upgrade to latest nan and address deprecations #146

Merged
merged 3 commits into from
Feb 6, 2019
Merged

Conversation

shiftkey
Copy link
Contributor

@shiftkey shiftkey commented Dec 28, 2018

Fixes #131

  • upgrade to latest nan
  • address any reported deprecations
  • get feedback on Utf8Value changes

@shiftkey shiftkey changed the title upgrade to latest nan and address deprecations [WIP] upgrade to latest nan and address deprecations Dec 28, 2018
@@ -4,40 +4,63 @@
namespace {

NAN_METHOD(SetPassword) {
Nan::Utf8String serviceNan(info[0]);
std::string service(*serviceNan, serviceNan.length());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nan has it's own UTF8 string abstraction that works across all the versions of v8, instead of #ifdef-ing across all the different Utf8Value versions that exist across different V8 versions, and this was the simplest way I could find to convert it into an array of bytes to then pass through to the platform-specific code.

If anyone knows a more succinct way to achieve this please let me know.

@shiftkey shiftkey changed the title [WIP] upgrade to latest nan and address deprecations upgrade to latest nan and address deprecations Dec 28, 2018
@shiftkey shiftkey requested a review from daviwil December 28, 2018 14:03
@shiftkey shiftkey force-pushed the upgrade-nan-usage branch 3 times, most recently from 707532e to f30203f Compare February 6, 2019 11:02
@shiftkey
Copy link
Contributor Author

shiftkey commented Feb 6, 2019

This has sat for a while without feedback, and given the tests all pass I plan to ship this in an update, probably v4.4.0, so we can confirm there are no problems with it in the wild.

@shiftkey shiftkey merged commit 22301ce into master Feb 6, 2019
@shiftkey shiftkey deleted the upgrade-nan-usage branch February 6, 2019 11:26
shiftkey referenced this pull request Feb 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant