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

src: increment NODE_MODULE_VERSION to 46 #2678

Closed
wants to merge 1 commit into from

Conversation

ofrobots
Copy link
Contributor

@ofrobots ofrobots commented Sep 3, 2015

Trying to get this into vee-eight-4.5 before the PR to master.

V8 4.5 C++ API changes that warrant this:

  • Remove deprecated v8::Object::TurnOnAccessCheck API
  • Remove obsolete options in ScriptCompiler::CompileOptions
  • Remove v8::Private

R=@rvagg, @nodejs/v8

V8 4.5 API changes that warrant this:
* Remove deprecated v8::Object::TurnOnAccessCheck API
* Remove obsolete options in ScriptCompiler::CompileOptions
* Remove v8::Private
Full details at https://docs.google.com/document/d/1g8JFi8T_oAE_7uAri7Njtig7fKaPDfotU6huOa1alds/edit?hl=en&forcehl=1
@ofrobots ofrobots added the v8 engine Issues and PRs related to the V8 dependency. label Sep 3, 2015
@ofrobots ofrobots added this to the 4.0.0 milestone Sep 3, 2015
@targos
Copy link
Member

targos commented Sep 3, 2015

LGTM

ofrobots added a commit that referenced this pull request Sep 3, 2015
V8 4.5 API changes that warrant this:
* Remove deprecated v8::Object::TurnOnAccessCheck API
* Remove obsolete options in ScriptCompiler::CompileOptions
* Remove v8::Private
Full details at https://docs.google.com/document/d/1g8JFi8T_oAE_7uAri7Njtig7fKaPDfotU6huOa1alds/edit?hl=en&forcehl=1

PR-URL: #2678
Reviewed-By: targos - Michaël Zasso <[email protected]>
@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 3, 2015

Landed in vee-eight-4.5 as b3d79b9.

@Fishrock123 Fishrock123 closed this Sep 3, 2015
@ofrobots ofrobots deleted the ofrobots-patch-1 branch September 4, 2015 00:07
@rvagg
Copy link
Member

rvagg commented Sep 4, 2015

Hmmm, normally we'd do this as part of the stable release process rather than coupling it with landing V8. We're going to have to figure out how this fits in with our master/canary/unstable "release" process, I for one don't want to see this number incremented every time a semver-major lands in master.

@Fishrock123
Copy link
Contributor

Yeah it's probably best to leave this to @nodejs/release

@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 4, 2015

Acknowledged. It would simple to drop this commit from #2682 before merging to master. LMK.

@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 4, 2015

Dropped from #2682 and vee-eight-4.5.

@thefourtheye
Copy link
Contributor

@ofrobots Can we remove vee-eight-4.5 branch now?

@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 4, 2015

@thefourtheye Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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