Skip to content

fix compile bug in v8/lookup.h#10525

Closed
matthewaveryusa wants to merge 2 commits intonodejs:v6.x-stagingfrom
matthewaveryusa:patch-1
Closed

fix compile bug in v8/lookup.h#10525
matthewaveryusa wants to merge 2 commits intonodejs:v6.x-stagingfrom
matthewaveryusa:patch-1

Conversation

@matthewaveryusa
Copy link

@matthewaveryusa matthewaveryusa commented Dec 29, 2016 •
edited
Loading

Fixed a small error that manifests when --debug is specified. Seems to be fixed in 7.x when v8 was pulled from upstream.
Sorry if this is the wrong branch but it doesn't apply to master as this is for the 6.x series.

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

build

@nodejs-github-bot nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label Dec 29, 2016
@MylesBorins
Copy link
Contributor

MylesBorins commented Dec 29, 2016 •
edited
Loading

@matthewaveryusa changes like this should be submitted upstream to V8 and be backported once the change lands there.

/cc @nodejs/v8

edit: Ahhhh this is already fixed upstream. So like the best thing to do would be to find the fix upstream and backport that change following the instructions found in https://github.com/nodejs/node/blob/master/doc/guides/maintaining-V8.md

edit-2: that change should then target the v6.x-staging branch fwiw

@MylesBorins
Copy link
Contributor

MylesBorins commented Dec 29, 2016

fwiw here is the original change from the V8 tracker v8/v8@5c8cb16

@ofrobots
Copy link
Contributor

ofrobots commented Dec 29, 2016

Ref: #9422.

matthewaveryusa and others added 2 commits December 29, 2016 14:08
@matthewaveryusa @ofrobots
deps: fix compile bug in v8/lookup.h
516b627 
Fixed a small error that manifests when --debug is specified. This
seems to have been introduced during the backport nodejs#9422.

Ref: nodejs#9422
PR-URL: nodejs#10525
@ofrobots
[SQUASH] bump v8 version
e74def5
@ofrobots ofrobots changed the base branch from v6.9.3-proposal to v6.x-staging December 29, 2016 22:10
@ofrobots
Copy link
Contributor

ofrobots commented Dec 29, 2016

Thanks for fixing the issue. LGTM.

I've added a V8 version bump commit on your branch, reworded the commit message for clarity/style, rebased to v6.x-staging, and retargeted the PR to v6.x-staging.

@gibfahn gibfahn mentioned this pull request Dec 30, 2016
Closed
3 tasks
@ofrobots
Copy link
Contributor

ofrobots commented Jan 3, 2017

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

mhdawson
mhdawson approved these changes Jan 3, 2017
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 if both CI's are green

ofrobots pushed a commit that referenced this pull request Jan 4, 2017
@matthewaveryusa @ofrobots
deps: fix compile bug in v8/lookup.h
1859558 
Fixed a small error that manifests when --debug is specified. This
seems to have been introduced during the backport #9422.

Ref: #9422
PR-URL: #10525
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com>
@ofrobots
Copy link
Contributor

ofrobots commented Jan 4, 2017

The tests were green enough.

Landed as 1859558 on v6.x-staging.

@ofrobots ofrobots closed this Jan 4, 2017
MylesBorins pushed a commit that referenced this pull request Jan 5, 2017
@matthewaveryusa @MylesBorins
deps: fix compile bug in v8/lookup.h
463addc 
Fixed a small error that manifests when --debug is specified. This
seems to have been introduced during the backport #9422.

Ref: #9422
PR-URL: #10525
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com>
@addaleax addaleax mentioned this pull request Jan 21, 2017
Closed
4 tasks
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
@matthewaveryusa @MylesBorins
deps: fix compile bug in v8/lookup.h
182e042 
Fixed a small error that manifests when --debug is specified. This
seems to have been introduced during the backport #9422.

Ref: #9422
PR-URL: #10525
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2017
Merged
@bnoordhuis bnoordhuis mentioned this pull request Jan 28, 2017
Closed
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
@matthewaveryusa @MylesBorins
deps: fix compile bug in v8/lookup.h
fdc373d 
Fixed a small error that manifests when --debug is specified. This
seems to have been introduced during the backport #9422.

Ref: #9422
PR-URL: #10525
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com>
@mscdex mscdex mentioned this pull request Mar 21, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@targos targos targos approved these changes

@mhdawson mhdawson mhdawson approved these changes

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.

6 participants

@matthewaveryusa @MylesBorins @ofrobots @targos @mhdawson @nodejs-github-bot