Skip to content

Conversation

@kfarnung
Copy link
Contributor

@kfarnungkfarnung commented Jul 13, 2017

The v8::PersistentBase<T>.Get method didn't exist in node 4 and it's
just a helper which creates a new v8::Local from the given object.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

n-api

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API. labels Jul 13, 2017
@kfarnung
Copy link
ContributorAuthor

Copy link
Member

@mhdawsonmhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@kfarnung
Copy link
ContributorAuthor

@kfarnung
Copy link
ContributorAuthor

kfarnung commented Jul 17, 2017

Rebased again with new CI: https://ci.nodejs.org/job/node-test-pull-request/9233/

@kfarnung
Copy link
ContributorAuthor

This should be ready to land, from what I can see the CI shows only known issues.

/cc @mhdawson@jasongin

@kfarnung
Copy link
ContributorAuthor

The `v8::PersistentBase<T>.Get` method didn't exist in node 4 and it's just a helper which creates a new `v8::Local` from the given object.
@kfarnung
Copy link
ContributorAuthor

Rebased and started a new CI: https://ci.nodejs.org/job/node-test-pull-request/9359/

@addaleax
Copy link
Member

Landed in e59987c, thanks!

addaleax pushed a commit that referenced this pull request Jul 26, 2017
The `v8::PersistentBase<T>.Get` method didn't exist in node 4 and it's just a helper which creates a new `v8::Local` from the given object. PR-URL: #14211 Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Jason Ginchereau <[email protected]>
@kfarnung
Copy link
ContributorAuthor

Thanks @addaleax!

@kfarnungkfarnung deleted the objtemplate branch July 26, 2017 18:27
addaleax pushed a commit that referenced this pull request Jul 27, 2017
The `v8::PersistentBase<T>.Get` method didn't exist in node 4 and it's just a helper which creates a new `v8::Local` from the given object. PR-URL: #14211 Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Jason Ginchereau <[email protected]>
@addaleaxaddaleax mentioned this pull request Aug 2, 2017
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 10, 2018
The `v8::PersistentBase<T>.Get` method didn't exist in node 4 and it's just a helper which creates a new `v8::Local` from the given object. PR-URL: nodejs#14211 Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Jason Ginchereau <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 16, 2018
The `v8::PersistentBase<T>.Get` method didn't exist in node 4 and it's just a helper which creates a new `v8::Local` from the given object. Backport-PR-URL: #19447 PR-URL: #14211 Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Jason Ginchereau <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Apr 16, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.node-apiIssues and PRs related to the Node-API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@kfarnung@addaleax@jasnell@gabrielschulhof@TimothyGu@cjihrig@mhdawson@jasongin@MylesBorins@nodejs-github-bot