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

(vee-eight-4.9) replace usage of deprecated V8 APIs #5159

Closed
wants to merge 15 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Feb 9, 2016

I did not touch SetWeak and SetAccessor yet (will need help for those).

Ref: #4869

cc @nodejs/v8

@targos targos added c++ Issues and PRs that require attention from people who are familiar with C++. v8 engine Issues and PRs related to the V8 dependency. labels Feb 9, 2016
@bnoordhuis
Copy link
Member

LGTM with style suggestions. Good work.

@targos
Copy link
Member Author

targos commented Feb 10, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/1625/

Should I squash everything into one commit (and list the changes in the message) before landing ?

@bnoordhuis
Copy link
Member

Whatever you prefer. I'd be okay with keeping the commits separate, might be easier if we need to bisect.

targos added a commit that referenced this pull request Feb 11, 2016
targos added a commit that referenced this pull request Feb 11, 2016
targos added a commit that referenced this pull request Feb 11, 2016
targos added a commit that referenced this pull request Feb 11, 2016
targos added a commit that referenced this pull request Feb 11, 2016
targos added a commit that referenced this pull request Feb 11, 2016
PR-URL: #5159
Reviewed-By: Ben Noordhuis <[email protected]>
targos added a commit that referenced this pull request Feb 11, 2016
targos added a commit that referenced this pull request Feb 11, 2016
targos added a commit that referenced this pull request Feb 11, 2016
targos added a commit that referenced this pull request Feb 11, 2016
ofrobots pushed a commit that referenced this pull request Mar 4, 2016
@Fishrock123
Copy link
Contributor

To confirm, is this safe to land in v5.x? (Please reply in #5559)

Sign up for free to 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++. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants