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.

Sign up for GitHub

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

Jump to bottom

deps: cherry-pick aa6ce3e from upstream V8 #21126

Merged
merged 4 commits into from
Jun 22, 2018
Merged

deps: cherry-pick aa6ce3e from upstream V8 #21126

merged 4 commits into from
Jun 22, 2018

Conversation

mmarchini
Copy link
Contributor

Original commit message:

[log][api] introduce public CodeEventListener API

Introduce a new public API called CodeEventListener to allow embedders
to better support external profilers and other diagnostic tools without
relying on unsupported methods like --perf-basic-prof.

Bug: v8:7694
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I063cc965394d59401358757634c9ea84c11517e9
Co-authored-by: Daniel Beckert <daniel@sthima.com.br>
Reviewed-on: chromium-review.googlesource.com/1028770
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53382}

Refs: v8/v8@aa6ce3e

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency. labels Jun 4, 2018
@mmarchini
Copy link
Contributor Author

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

@mmarchini
Copy link
Contributor Author

cc @nodejs/diagnostics

@targos
Copy link
Member

targos commented Jun 4, 2018

The fixup isn't upstream?

@mmarchini
Copy link
Contributor Author

The fixup isn't upstream?

The patch didn't build because the Address typedef is different between V8 6.7 and V8 6.8 (see https://chromium-review.googlesource.com/c/v8/v8/+/988657). I can try to float https://chromium-review.googlesource.com/c/v8/v8/+/988657 as well instead of changing the Patch to work on V8 6.7.

@targos
Copy link
Member

targos commented Jun 4, 2018

I can try to float chromium-review.googlesource.com/c/v8/v8/+/988657 as well instead of changing the Patch to work on V8 6.7.

It's a big patch. I prefer we stick with your fix.

@mmarchini
Copy link
Contributor Author

Floating https://chromium-review.googlesource.com/c/v8/v8/+/988657 is non-trivial :(

Error: Command failed: git apply -3 --directory=deps/v8
error: patch failed: deps/v8/src/compiler/wasm-compiler.cc:2619
error: repository lacks the necessary blob to fall back on 3-way merge.
error: deps/v8/src/compiler/wasm-compiler.cc: patch does not apply
error: patch failed: deps/v8/src/log.cc:309
Falling back to three-way merge...
Applied patch to 'deps/v8/src/log.cc' cleanly.
error: patch failed: deps/v8/src/profiler/profile-generator.h:47
error: repository lacks the necessary blob to fall back on 3-way merge.
error: deps/v8/src/profiler/profile-generator.h: patch does not apply
error: patch failed: deps/v8/src/profiler/profiler-listener.h:59
error: repository lacks the necessary blob to fall back on 3-way merge.
error: deps/v8/src/profiler/profiler-listener.h: patch does not apply
error: patch failed: deps/v8/test/cctest/test-code-stubs-arm.cc:53
error: repository lacks the necessary blob to fall back on 3-way merge.
error: deps/v8/test/cctest/test-code-stubs-arm.cc: patch does not apply
error: patch failed: deps/v8/test/cctest/test-code-stubs-arm64.cc:54
error: repository lacks the necessary blob to fall back on 3-way merge.
error: deps/v8/test/cctest/test-code-stubs-arm64.cc: patch does not apply
error: patch failed: deps/v8/test/cctest/test-code-stubs-ia32.cc:54
error: repository lacks the necessary blob to fall back on 3-way merge.
error: deps/v8/test/cctest/test-code-stubs-ia32.cc: patch does not apply
error: patch failed: deps/v8/test/cctest/test-code-stubs-mips.cc:55
error: repository lacks the necessary blob to fall back on 3-way merge.
error: deps/v8/test/cctest/test-code-stubs-mips.cc: patch does not apply
error: patch failed: deps/v8/test/cctest/test-code-stubs-mips64.cc:55
error: repository lacks the necessary blob to fall back on 3-way merge.
error: deps/v8/test/cctest/test-code-stubs-mips64.cc: patch does not apply
error: patch failed: deps/v8/test/cctest/test-code-stubs-x64.cc:54
error: repository lacks the necessary blob to fall back on 3-way merge.
error: deps/v8/test/cctest/test-code-stubs-x64.cc: patch does not apply

For context: this API will allow users to generate the map file required by Linux perf during runtime. Today the file is generated by --perf-basic-prof, which will keep writing to the file during the entire application lifetime, leading to file growth problems. Sice V8 6.7 fixed Linux perf for Interpreted Functions, it would be really nice to also have this API when V8 6.7 lands on Node.js v10.x.

@hashseed
Copy link
Member

hashseed commented Jun 5, 2018

Yeah let's not float the Address patch.

@mmarchini
Copy link
Contributor Author

mmarchini commented Jun 11, 2018 •
edited
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/15393/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/1415/
V8 CI rerun: https://ci.nodejs.org/job/node-test-commit-v8-linux/1416/

@mmarchini
Copy link
Contributor Author

Cherry-picking one more commit here (v8/v8@b20faff) to fix a bug when using this API with --interpreted-frames-native-stack on. If there are no objections I'd like to land this tomorrow.

/cc @nodejs/diagnostics @nodejs/v8-update

@targos
Copy link
Member

targos commented Jun 11, 2018

There is an issue with the v8 ci. Can you try to run it again?

@mmarchini
Copy link
Contributor Author

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

mhdawson
mhdawson approved these changes Jun 11, 2018
View reviewed changes
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.

LGTM

@mmarchini mmarchini added the blocked PRs that are blocked by other issues or PRs. label Jun 12, 2018
@mmarchini
Copy link
Contributor Author

mmarchini commented Jun 12, 2018 •
edited
Loading

Blocking due to another bug found in the API.

Waiting for upstream fix to land: https://chromium-review.googlesource.com/c/v8/v8/+/1097578

@mmarchini mmarchini mentioned this pull request Jun 13, 2018
Closed
@mmarchini mmarchini force-pushed the v8-float-code-event-listener-api branch from eaeceb9 to 20dc9b8 Compare June 18, 2018 20:35
@mmarchini
Copy link
Contributor Author

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

@mmarchini mmarchini force-pushed the v8-float-code-event-listener-api branch from 20dc9b8 to 348a2be Compare June 21, 2018 15:51
@mmarchini
Copy link
Contributor Author

@targos should I squash the fixup when landing, or sould I keep it as a separate commit?

@targos
Copy link
Member

targos commented Jun 21, 2018

Squash please and write "backport" instead of cherry-pick in the commit message

@mmarchini mmarchini force-pushed the v8-float-code-event-listener-api branch from 348a2be to f43a71b Compare June 21, 2018 22:39
@mmarchini
Copy link
Contributor Author

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

@mmarchini mmarchini added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed blocked PRs that are blocked by other issues or PRs. labels Jun 22, 2018
Matheus Marchini added 2 commits June 22, 2018 09:42
deps: backport aa6ce3e from upstream V8
b362756 
Original commit message:

    [log][api] introduce public CodeEventListener API

    Introduce a new public API called CodeEventListener to allow
    embedders to better support external profilers and other diagnostic
    tools without relying on unsupported methods like --perf-basic-prof.

    Bug: v8:7694
    Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
    Change-Id: I063cc965394d59401358757634c9ea84c11517e9
    Co-authored-by: Daniel Beckert <daniel@sthima.com.br>
    Reviewed-on: https://chromium-review.googlesource.com/1028770
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Hannes Payer <hpayer@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Andreas Haas <ahaas@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#53382}

Refs: v8/v8@aa6ce3e

PR-URL: nodejs#21126
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
deps: cherry-pick b20faff from upstream V8
9bdc882 
Original commit message:

    [log] fix ExistingCodeLogger behavior on edge case

    ExistingCodeLogger was behaving incorrectly when the
    CodeEventHandler API was used in combination with
    --interpreted-frames-native-stack.  Instead of collecting copied
    trampolines as InterpretedFunction:functionName, they were being
    collected as Builtin:IntepreterEntryTrampolines.  This patch adds
    special handling for copied trampolines when using
    ExistingCodeLogger.

    R=yangguo@google.com

    Change-Id: I3ee4be03800122d28d53b51b20c60dcf6263e4c1
    Reviewed-on: https://chromium-review.googlesource.com/1087813
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#53624}

Refs: v8/v8@b20faff

PR-URL: nodejs#21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Jul 25, 2018
@targos
deps: cherry-pick b20faff from upstream V8
742f9ea 
Original commit message:

    [log] fix ExistingCodeLogger behavior on edge case

    ExistingCodeLogger was behaving incorrectly when the
    CodeEventHandler API was used in combination with
    --interpreted-frames-native-stack.  Instead of collecting copied
    trampolines as InterpretedFunction:functionName, they were being
    collected as Builtin:IntepreterEntryTrampolines.  This patch adds
    special handling for copied trampolines when using
    ExistingCodeLogger.

    R=yangguo@google.com

    Change-Id: I3ee4be03800122d28d53b51b20c60dcf6263e4c1
    Reviewed-on: https://chromium-review.googlesource.com/1087813
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#53624}

Refs: v8/v8@b20faff

PR-URL: nodejs#21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Jul 25, 2018
@targos
deps: cherry-pick acc336c from upstream V8
06c0358 
Original commit message:

    [log] fix boolean logic on LogCodeObject

    R=yangguo@google.com

    Change-Id: Icb4825344991e5b2d15050e037064c60eeb9617e
    Reviewed-on: https://chromium-review.googlesource.com/1097578
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#53777}

Refs: v8/v8@acc336c

PR-URL: nodejs#21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Jul 25, 2018
@targos
deps: cherry-pick 70c4340 from upstream V8
e39f302 
Original commit message:

    [log][api] Fix GCC 4.9 build failure

    GCC 4.9 used on some Node.js CI machines complains when the control
    reaches the end of a non-void function and no return is encountered.

    R=bmeurer@google.com, ofrobots@google.com, yangguo@google.com

    Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
    Change-Id: I5af0192cb187eccbf34dbb60ff3ac2e4774af803
    Reviewed-on: https://chromium-review.googlesource.com/1105619
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#53861}

Refs: v8/v8@70c4340

PR-URL: nodejs#21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Jul 26, 2018
@targos
deps: cherry-pick b20faff from upstream V8
76f4a5e 
Original commit message:

    [log] fix ExistingCodeLogger behavior on edge case

    ExistingCodeLogger was behaving incorrectly when the
    CodeEventHandler API was used in combination with
    --interpreted-frames-native-stack.  Instead of collecting copied
    trampolines as InterpretedFunction:functionName, they were being
    collected as Builtin:IntepreterEntryTrampolines.  This patch adds
    special handling for copied trampolines when using
    ExistingCodeLogger.

    R=yangguo@google.com

    Change-Id: I3ee4be03800122d28d53b51b20c60dcf6263e4c1
    Reviewed-on: https://chromium-review.googlesource.com/1087813
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#53624}

Refs: v8/v8@b20faff

PR-URL: nodejs#21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Jul 26, 2018
@targos
deps: cherry-pick acc336c from upstream V8
d15db82 
Original commit message:

    [log] fix boolean logic on LogCodeObject

    R=yangguo@google.com

    Change-Id: Icb4825344991e5b2d15050e037064c60eeb9617e
    Reviewed-on: https://chromium-review.googlesource.com/1097578
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#53777}

Refs: v8/v8@acc336c

PR-URL: nodejs#21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Jul 26, 2018
@targos
deps: cherry-pick 70c4340 from upstream V8
eea5aee 
Original commit message:

    [log][api] Fix GCC 4.9 build failure

    GCC 4.9 used on some Node.js CI machines complains when the control
    reaches the end of a non-void function and no return is encountered.

    R=bmeurer@google.com, ofrobots@google.com, yangguo@google.com

    Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
    Change-Id: I5af0192cb187eccbf34dbb60ff3ac2e4774af803
    Reviewed-on: https://chromium-review.googlesource.com/1105619
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#53861}

Refs: v8/v8@70c4340

PR-URL: nodejs#21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Jul 26, 2018
@targos
deps: cherry-pick b20faff from upstream V8
8c85164 
Original commit message:

    [log] fix ExistingCodeLogger behavior on edge case

    ExistingCodeLogger was behaving incorrectly when the
    CodeEventHandler API was used in combination with
    --interpreted-frames-native-stack.  Instead of collecting copied
    trampolines as InterpretedFunction:functionName, they were being
    collected as Builtin:IntepreterEntryTrampolines.  This patch adds
    special handling for copied trampolines when using
    ExistingCodeLogger.

    R=yangguo@google.com

    Change-Id: I3ee4be03800122d28d53b51b20c60dcf6263e4c1
    Reviewed-on: https://chromium-review.googlesource.com/1087813
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#53624}

Refs: v8/v8@b20faff

PR-URL: nodejs#21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Jul 26, 2018
@targos
deps: cherry-pick acc336c from upstream V8
309052a 
Original commit message:

    [log] fix boolean logic on LogCodeObject

    R=yangguo@google.com

    Change-Id: Icb4825344991e5b2d15050e037064c60eeb9617e
    Reviewed-on: https://chromium-review.googlesource.com/1097578
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#53777}

Refs: v8/v8@acc336c

PR-URL: nodejs#21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Jul 26, 2018
@targos
deps: cherry-pick 70c4340 from upstream V8
fad5afa 
Original commit message:

    [log][api] Fix GCC 4.9 build failure

    GCC 4.9 used on some Node.js CI machines complains when the control
    reaches the end of a non-void function and no return is encountered.

    R=bmeurer@google.com, ofrobots@google.com, yangguo@google.com

    Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
    Change-Id: I5af0192cb187eccbf34dbb60ff3ac2e4774af803
    Reviewed-on: https://chromium-review.googlesource.com/1105619
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#53861}

Refs: v8/v8@70c4340

PR-URL: nodejs#21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 10, 2018
@MylesBorins
deps: cherry-pick b20faff from upstream V8
5b6c401 
Original commit message:

    [log] fix ExistingCodeLogger behavior on edge case

    ExistingCodeLogger was behaving incorrectly when the
    CodeEventHandler API was used in combination with
    --interpreted-frames-native-stack.  Instead of collecting copied
    trampolines as InterpretedFunction:functionName, they were being
    collected as Builtin:IntepreterEntryTrampolines.  This patch adds
    special handling for copied trampolines when using
    ExistingCodeLogger.

    R=yangguo@google.com

    Change-Id: I3ee4be03800122d28d53b51b20c60dcf6263e4c1
    Reviewed-on: https://chromium-review.googlesource.com/1087813
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#53624}

Refs: v8/v8@b20faff

Backport-PR-URL: #21668
PR-URL: #21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 10, 2018
@MylesBorins
deps: cherry-pick acc336c from upstream V8
ffa835e 
Original commit message:

    [log] fix boolean logic on LogCodeObject

    R=yangguo@google.com

    Change-Id: Icb4825344991e5b2d15050e037064c60eeb9617e
    Reviewed-on: https://chromium-review.googlesource.com/1097578
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#53777}

Refs: v8/v8@acc336c

Backport-PR-URL: #21668
PR-URL: #21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 10, 2018
@MylesBorins
deps: cherry-pick 70c4340 from upstream V8
3afffeb 
Original commit message:

    [log][api] Fix GCC 4.9 build failure

    GCC 4.9 used on some Node.js CI machines complains when the control
    reaches the end of a non-void function and no return is encountered.

    R=bmeurer@google.com, ofrobots@google.com, yangguo@google.com

    Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
    Change-Id: I5af0192cb187eccbf34dbb60ff3ac2e4774af803
    Reviewed-on: https://chromium-review.googlesource.com/1105619
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#53861}

Refs: v8/v8@70c4340

Backport-PR-URL: #21668
PR-URL: #21126
Refs: v8/v8@aa6ce3e
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@rvagg rvagg mentioned this pull request Aug 13, 2018
Merged
This was referenced Aug 16, 2018
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhdawson mhdawson mhdawson approved these changes

@targos targos targos approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. 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.
6 participants
@mmarchini @targos @hashseed @jasnell @mhdawson @nodejs-github-bot