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

lib: include missing profiler file #18455

Closed
wants to merge 1 commit into from
Closed

lib: include missing profiler file #18455

wants to merge 1 commit into from

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Jan 30, 2018

This commit includes deps/v8/tools/arguments.js, which is needed by the profiler.

Fixes: #18451

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

lib

@cjihrig
lib: include missing profiler file
f43e53c 
This commit includes deps/v8/tools/arguments.js, which is
needed by the profiler.
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. labels Jan 30, 2018
bnoordhuis
bnoordhuis approved these changes Jan 30, 2018
View reviewed changes
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 if make test-tick-processor passes again.

@cjihrig
Copy link
Contributor Author

cjihrig commented Jan 31, 2018

The tick processor tests are flaky for me since the V8 6.3 update in #16271. The changes in this PR don't fix the flakiness, but it does make it run again. Things seem to run fine outside of the tests though.

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 1, 2018
@BridgeAR
Copy link
Member

BridgeAR commented Feb 1, 2018

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

@jasnell
Copy link
Member

jasnell commented Feb 2, 2018

CI should be good but trying again on arm just to be safe: https://ci.nodejs.org/job/node-test-commit-arm-fanned/14179/

jasnell pushed a commit that referenced this pull request Feb 2, 2018
@cjihrig @jasnell
lib: include missing profiler file
42258d7 
This commit includes deps/v8/tools/arguments.js, which is
needed by the profiler.

PR-URL: #18455
Fixes: #18451
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Feb 2, 2018

Landed in 42258d7

@jasnell jasnell closed this Feb 2, 2018
@cjihrig cjihrig deleted the 18451 branch February 3, 2018 04:00
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 4, 2018
@MylesBorins
Copy link
Contributor

The original bug does not seem to affect v9.x so setting as don't land. Please feel free to change this

/cc @nodejs/v8

MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@cjihrig @MayaLekova
lib: include missing profiler file
5dbfbcd 
This commit includes deps/v8/tools/arguments.js, which is
needed by the profiler.

PR-URL: nodejs#18455
Fixes: nodejs#18451
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
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

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@cjihrig @BridgeAR @jasnell @MylesBorins @bnoordhuis @addaleax @nodejs-github-bot