Skip to content

deps: update V8 to 5.4.500.41#9412

Merged
targos merged 1 commit intonodejs:masterfrom
targos:update-v8-40
Nov 11, 2016
Merged

deps: update V8 to 5.4.500.41#9412
targos merged 1 commit intonodejs:masterfrom
targos:update-v8-40

Conversation

@targos
Copy link
Copy Markdown
Member

@targos targos commented Nov 2, 2016 •
edited
Loading

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
bnoordhuis
bnoordhuis approved these changes Nov 2, 2016
View reviewed changes
Comment thread deps/v8/src/runtime/runtime-utils.h Outdated
Copy link
Copy Markdown
Member

@bnoordhuis bnoordhuis Nov 2, 2016

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
Copy Markdown
Contributor

MylesBorins commented Nov 2, 2016

V8 Ci: https://ci.nodejs.org/job/node-test-commit-v8-linux/389/

@fhinkel
Copy link
Copy Markdown
Member

fhinkel commented Nov 2, 2016

LGTM

cjihrig
cjihrig approved these changes Nov 2, 2016
View reviewed changes
Copy link
Copy Markdown
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

mhdawson
mhdawson approved these changes Nov 4, 2016
View reviewed changes
Copy link
Copy Markdown
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

@bricss bricss mentioned this pull request Nov 6, 2016
Merged
@targos
Copy link
Copy Markdown
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
Copy Markdown
Member Author

targos commented Nov 10, 2016

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

ofrobots
ofrobots approved these changes Nov 10, 2016
View reviewed changes
Copy link
Copy Markdown
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.

bnoordhuis
bnoordhuis approved these changes Nov 11, 2016
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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...

@targos
deps: update V8 to 5.4.500.41
0fa09b4 
PR-URL: nodejs#9412
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
@targos
Copy link
Copy Markdown
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
@targos @MylesBorins
deps: update V8 to 5.4.500.41
33bcd6f 
PR-URL: #9412
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jasnell jasnell jasnell approved these changes

@cjihrig cjihrig cjihrig approved these changes

+2 more reviewers

@ofrobots ofrobots ofrobots approved these changes

@mhdawson mhdawson mhdawson approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@targos @MylesBorins @fhinkel @ofrobots @bnoordhuis @jasnell @cjihrig @mhdawson