Skip to content

deps: adapt to V8 7.0#808

Merged
kkoopa merged 2 commits into
nodejs:masterfrom
refack:adapt-to-v8-70
Sep 29, 2018
Merged

deps: adapt to V8 7.0#808
kkoopa merged 2 commits into
nodejs:masterfrom
refack:adapt-to-v8-70

Conversation

@refack

@refack refack commented Sep 28, 2018

Copy link
Copy Markdown
Contributor

Fixes: nodejs/node#23122

Also updates dependencies

addaleax

This comment was marked as off-topic.

@addaleax

Copy link
Copy Markdown
Member

@rvagg @kkoopa Can we get a release out with this asap? I think that would be a really good step forward for un-breaking CITGM…

@kkoopa kkoopa merged commit 24a22c3 into nodejs:master Sep 29, 2018
@refack refack deleted the adapt-to-v8-70 branch September 29, 2018 15:18
@chjj

chjj commented Nov 3, 2018

Copy link
Copy Markdown

Still receiving deprecation warnings with 2.11.1:

../../nan/nan.h:1064:78: warning: v8::Local<v8::String> v8::Value::ToString(v8::Isolate*) const is deprecated: Use maybe version [-Wdeprecated-declarations]
       v8::Local<v8::String> string = from->ToString(v8::Isolate::GetCurrent());

It appears that:

v8::Local<v8::String> v8::Value::ToString(v8::Isolate*) const

is also deprecated.

Shouldn't it be doing something like:

v8::Local<v8::String> string = from->ToString(v8::Isolate::GetCurrent()->GetCurrentContext()).FromMaybe(v8::Local<v8::String>());

Not sure.

cc @addaleax

@Flarna

Flarna commented Nov 3, 2018

Copy link
Copy Markdown
Member

@chjj I think #811 addresses these warnings

@chjj

chjj commented Nov 3, 2018

Copy link
Copy Markdown

@Flarna, ah, thanks. Didn't see that.

All this v8 stuff is driving me nuts. I should really switch to NAPI.

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