Skip to content

deps: update v8_inspector#8014

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
Merged
ofrobots merged 1 commit into from
Aug 11, 2016
Merged

deps: update v8_inspector #8014

ofrobots merged 1 commit into from
Aug 11, 2016

Conversation

@ofrobots
Copy link
Contributor

@ofrobotsofrobots commented Aug 8, 2016•
edited
Loading

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

Update to the latest v8_inspector (pavelfeldman/v8_inspector@1352de2).

@nodejs-github-botnodejs-github-bot added the inspector Issues and PRs related to the V8 inspector protocol label Aug 8, 2016
@ofrobotsofrobots mentioned this pull request Aug 8, 2016
Closed
4 tasks
@bnoordhuis
Copy link
Member

bnoordhuis commented Aug 8, 2016

Changes to src/ LGTM and rubber-stamp LGTM w.r.t. everything else.

@ofrobots
Copy link
ContributorAuthor

ofrobots commented Aug 8, 2016

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

@ofrobots
Copy link
ContributorAuthor

ofrobots commented Aug 10, 2016

Updated this PR to pick up a more recent version from upstream, given that it is needed for #8043. Slight changes to the code in src/, so this needs another look.

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

@ofrobots
Copy link
ContributorAuthor

ofrobots commented Aug 10, 2016

One more roll to pick the fix needed for v6.x compatibility. New CI: https://ci.nodejs.org/job/node-test-pull-request/3615/

@ofrobotsofrobots mentioned this pull request Aug 10, 2016
Closed
@cjihrig
Copy link
Contributor

cjihrig commented Aug 10, 2016

Was the hard coded "NodeJS Main Context" part of the original changes that got signed off on? If so, LGTM and the CI is green.

@ofrobots
Copy link
ContributorAuthor

ofrobots commented Aug 11, 2016

Was the hard coded "NodeJS Main Context" part of the original changes that got signed off on? If so, LGTM and the CI is green.

Yes, that was part of the original changes. The context name, "NodeJS Main Context", used to be hardcoded in the upstream v8_inspector code but now the embedder can provide the name.

The CI is green. I will go ahead and land this in a couple of hours.

@ofrobots
deps: v8_inspector update
e40234d
Pick up latest from [1] corresponding to the Blink commit 62cd277. [1]: pavelfeldman/v8_inspector@e6b8355 PR-URL: nodejs#8014 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]> Reviewed-By: cjihrig - Colin Ihrig <[email protected]>
@ofrobotsofrobots merged commit e40234d into nodejs:masterAug 11, 2016
cjihrig pushed a commit that referenced this pull request Aug 11, 2016
@ofrobots@cjihrig
deps: v8_inspector update
a9fe85e
Pick up latest from [1] corresponding to the Blink commit 62cd277. [1]: pavelfeldman/v8_inspector@e6b8355 PR-URL: #8014 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]> Reviewed-By: cjihrig - Colin Ihrig <[email protected]>
@cjihrigcjihrig mentioned this pull request Aug 11, 2016
Merged
@ofrobotsofrobots deleted the update-inspector branch August 11, 2016 20:28
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

inspectorIssues and PRs related to the V8 inspector protocol

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ofrobots@bnoordhuis@cjihrig@MylesBorins@nodejs-github-bot