Skip to content

Conversation

@MylesBorins
Copy link
Contributor

@MylesBorins MylesBorins commented May 7, 2017

This is a patch to make V8 5.8 ABI compatible with V8 5.9

I did not include a patch to bump the NODE_MODULES version. Do we plan to bump it for each ABI change prior to release?

V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/690/

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

@nodejs-github-bot nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label May 7, 2017
@MylesBorins MylesBorins requested review from addaleax, jasnell and targos May 7, 2017 09:58
@addaleax
Copy link
Member

addaleax commented May 7, 2017

Comparing with my own attempt in #12875, this is missing:

  • c107737 – fixes API, not ABI, breakage, and includes a deprecation that we probably want
  • 01605c0 – ABI breakage, that was just overlooked I guess?
  • 2f6c4bd – as long as we consider deprecations API breakage, this is needed
  • ac5c950 – also ABI breakage that seems to have been overlooked in this patch?

I would say that this patch has a somewhat more elegant solution for the changed value of kJSApiObjectType, but other than that #12875 seems more complete right now.

@addaleax addaleax added this to the 8.0.0 milestone May 7, 2017
@MylesBorins
Copy link
Contributor Author

hey @addaleax didn't see your original patch. Want me to close this?

@addaleax
Copy link
Member

addaleax commented May 7, 2017

@MylesBorins I don’t really care that much… If you want, feel free to cherry-pick the changes listed above (plus 932bd46 and maybe cbf3ecf) to this PR and I’ll close mine. ¯\_(ツ)_/¯

@MylesBorins
Copy link
Contributor Author

Closing in favor of #12875

@MylesBorins MylesBorins closed this May 7, 2017
@Trott Trott removed this from the 8.0.0 milestone May 7, 2017
@targos targos mentioned this pull request Jun 7, 2017
2 tasks
@MylesBorins MylesBorins deleted the 5.8-w-5.9-abi branch November 14, 2017 17:43
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.

4 participants