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

Updated NAN to 2.14 #43

Merged
merged 3 commits into from
Dec 3, 2019
Merged

Updated NAN to 2.14 #43

merged 3 commits into from
Dec 3, 2019

Conversation

mielvds
Copy link
Collaborator

@mielvds mielvds commented Dec 3, 2019

Fixes #41 and makes HDT-Node compatible with Node 12.
Not sure about the backwards compatibility now, to be tested.

Copy link
Member

@RubenVerborgh RubenVerborgh left a comment

Choose a reason for hiding this comment

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

Thanks! But can we also update .travis.yml to include 11, 12, 13, lts/*, node/*? (cfr. https://github.com/rdfjs/N3.js/blob/7bbfb84e2d650edf896792c76ad7da1e5436fa3c/.travis.yml)

lib/HdtDocument.cc Outdated Show resolved Hide resolved
@mielvds
Copy link
Collaborator Author

mielvds commented Dec 3, 2019

Thanks! But can we also update .travis.yml to include 11, 12, 13, lts/*, node/*? (cfr. https://github.com/rdfjs/N3.js/blob/7bbfb84e2d650edf896792c76ad7da1e5436fa3c/.travis.yml)

"node/*" or "node"?

@RubenVerborgh
Copy link
Member

Both.

@mielvds
Copy link
Collaborator Author

mielvds commented Dec 3, 2019

Ok, looks like they removed Set in Node 13.
Before it's:

../lib/HdtDocument.cc:209:21: warning: 'Set' is deprecated: Use maybe version [-Wdeprecated-declarations]
      triplesArray->Set(count++, tripleObject);

@RubenVerborgh
Copy link
Member

Deprecation is fine for now; unless NAN already has an alternative.

@mielvds
Copy link
Collaborator Author

mielvds commented Dec 3, 2019

They do: https://github.com/nodejs/nan/blob/master/doc/maybe_types.md#nanset I'll see if it's much work

@mielvds
Copy link
Collaborator Author

mielvds commented Dec 3, 2019

latest commit enables building for 13 as well

@RubenVerborgh
Copy link
Member

Okay the node/* was my bad; will fix. Looks good otherwise.

@RubenVerborgh RubenVerborgh merged commit 6f9ca64 into LinkedDataFragments:master Dec 3, 2019
@mielvds mielvds deleted the feature-upgrade-nan branch December 9, 2019 14:35
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.

Nan API may have changed in Node 12
2 participants