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

deps: update V8 to 5.4.500.41 #9412

Merged
merged 1 commit into from
Nov 11, 2016
Merged

deps: update V8 to 5.4.500.41 #9412

merged 1 commit into from
Nov 11, 2016

Conversation

targos
Copy link
Member

@targos targos commented Nov 2, 2016

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

V8

Description of change

Update V8 to the latest patch version.
/cc @nodejs/v8

Diff: v8/v8@5.4.500.36...5.4.500.40
CI: https://ci.nodejs.org/job/node-test-pull-request/4760/

@targos targos added the v8 engine Issues and PRs related to the V8 dependency. label Nov 2, 2016
int32_t __tmp_##name = 0; \
CHECK(args[index]->ToInt32(&__tmp_##name)); \
CHECK(is_valid_language_mode(__tmp_##name)); \
LanguageMode name = static_cast<LanguageMode>(__tmp_##name);
Copy link
Member

Choose a reason for hiding this comment

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

I confess to not understanding why this change is necessary. I'm going to guess it's got something to do with the InitializeVarGlobal intrinsic but the details elude me.

@MylesBorins
Copy link
Contributor

@fhinkel
Copy link
Member

fhinkel commented Nov 2, 2016

LGTM

Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

Rubber stamp LGTM

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

Rubber stamp LGTM

@targos
Copy link
Member Author

targos commented Nov 10, 2016

I was waiting for the V8 CI job to work again.
Just incremented the update to 5.4.500.41.

CI: https://ci.nodejs.org/job/node-test-pull-request/4817/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/404/

@targos targos changed the title deps: update V8 to 5.4.500.40 deps: update V8 to 5.4.500.41 Nov 10, 2016
@targos
Copy link
Member Author

targos commented Nov 10, 2016

CI is green. Can I have a few LGTM for the new commit ?

Copy link
Contributor

@ofrobots ofrobots left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM. I assume you are you going to squash the commits?

@targos
Copy link
Member Author

targos commented Nov 11, 2016

I assume you are you going to squash the commits?

That's the plan. Going to land now...

PR-URL: nodejs#9412
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@targos
Copy link
Member Author

targos commented Nov 11, 2016

Landed in 0fa09b4

@targos targos merged commit 0fa09b4 into nodejs:master Nov 11, 2016
@targos targos deleted the update-v8-40 branch November 11, 2016 09:42
MylesBorins pushed a commit that referenced this pull request Nov 11, 2016
PR-URL: #9412
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
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.

8 participants