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

fix compile bug in v8/lookup.h #10525

Closed
wants to merge 2 commits into from
Closed

fix compile bug in v8/lookup.h #10525

wants to merge 2 commits into from

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

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

@ofrobots
Copy link
Contributor

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

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 <[email protected]>
Reviewed-By: mhdawson - Michael Dawson <[email protected]>
@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 <[email protected]>
Reviewed-By: mhdawson - Michael Dawson <[email protected]>
@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 <[email protected]>
Reviewed-By: mhdawson - Michael Dawson <[email protected]>
@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 <[email protected]>
Reviewed-By: mhdawson - Michael Dawson <[email protected]>
@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

@mhdawson mhdawson mhdawson approved these changes

@targos targos targos 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