FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src,test: Prepare for future V8 changes by targos · Pull Request #11437 · nodejs/node · GitHub

/ node Public

src,test: Prepare for future V8 changes - #11437

Merged
targos merged 2 commits into
nodejs:masterfrom
targos:v8-changes
Feb 19, 2017
ConversationCommits (2)ChecksFiles changed
Merged

src,test: Prepare for future V8 changes#11437
targos merged 2 commits into
nodejs:masterfrom
targos:v8-changes

Conversation

targos commented Feb 17, 2017

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

/cc @nodejs/v8 @ofrobots @hashseed

targos added lib / src Issues and PRs related to general changes in the lib or src directory. test Issues and PRs related to the tests. v8 engine Issues and PRs related to the V8 dependency. labels Feb 17, 2017
nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. vm Issues and PRs related to the vm subsystem. labels Feb 17, 2017
addaleax approved these changes Feb 17, 2017
View reviewed changes

addaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM with a green CI

bnoordhuis approved these changes Feb 17, 2017
View reviewed changes
Comment thread src/node_contextify.cc Outdated

bnoordhuis Feb 17, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

You might as well use a lambda to put things closer together:

auto dummy_event_listener = [] (const Debug::EventDetails&) {};
Debug::SetDebugEventListener(args.GetIsolate(), dummy_event_listener);

Not a deal-breaker, though.

targos Feb 17, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is it supported by all compilers?

bnoordhuis Feb 17, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, it's standard C++11 syntax.

targos Feb 17, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Changed

targos commented Feb 17, 2017

Copy link
Copy Markdown
Member Author

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

mhdawson approved these changes Feb 17, 2017
View reviewed changes

mhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

hashseed and others added 2 commits February 19, 2017 10:17
src: remove usage of deprecated debug API
99b75fa 
Debug::GetMirror is used in contextify to force loading of the debug
context. There is a better way to achieve this.

PR-URL: nodejs#11437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
test: fix over-dependence on native promise impl
751404d 
In later versions of V8, the stack trace will be different than our
current expectations for Promise methods.

PR-URL: nodejs#11437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

targos commented Feb 19, 2017

Copy link
Copy Markdown
Member Author

Landed in 99b75fa and 751404d

targos closed this Feb 19, 2017
targos deleted the v8-changes branch February 19, 2017 09:24
targos merged commit 751404d into nodejs:master Feb 19, 2017
addaleax pushed a commit that referenced this pull request Feb 22, 2017
src: remove usage of deprecated debug API
539e83a 
Debug::GetMirror is used in contextify to force loading of the debug
context. There is a better way to achieve this.

PR-URL: #11437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
addaleax pushed a commit that referenced this pull request Feb 22, 2017
test: fix over-dependence on native promise impl
1d86a9f 
In later versions of V8, the stack trace will be different than our
current expectations for Promise methods.

PR-URL: #11437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
italoacasas mentioned this pull request Feb 25, 2017
Merged
jasnell pushed a commit that referenced this pull request Mar 7, 2017
test: fix over-dependence on native promise impl
170cb3c 
In later versions of V8, the stack trace will be different than our
current expectations for Promise methods.

PR-URL: #11437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
jasnell pushed a commit that referenced this pull request Mar 7, 2017
src: remove usage of deprecated debug API
8ca88c5 
Debug::GetMirror is used in contextify to force loading of the debug
context. There is a better way to achieve this.

PR-URL: #11437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
test: fix over-dependence on native promise impl
0ddad76 
In later versions of V8, the stack trace will be different than our
current expectations for Promise methods.

PR-URL: #11437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
src: remove usage of deprecated debug API
1dddfec 
Debug::GetMirror is used in contextify to force loading of the debug
context. There is a better way to achieve this.

PR-URL: #11437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins mentioned this pull request Mar 9, 2017
Merged
bbondy pushed a commit to brave/node that referenced this pull request Mar 22, 2017
src: remove usage of deprecated debug API
724d562 
Debug::GetMirror is used in contextify to force loading of the debug
context. There is a better way to achieve this.

PR-URL: nodejs/node#11437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
zcbenz pushed a commit to electron/node that referenced this pull request Apr 4, 2017
src: remove usage of deprecated debug API
3ba5cfb 
Debug::GetMirror is used in contextify to force loading of the debug
context. There is a better way to achieve this.

PR-URL: nodejs/node#11437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
bbondy pushed a commit to brave/node that referenced this pull request Apr 9, 2017
src: remove usage of deprecated debug API
b88e848 
Debug::GetMirror is used in contextify to force loading of the debug
context. There is a better way to achieve this.

PR-URL: nodejs/node#11437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
zcbenz pushed a commit to electron/node that referenced this pull request Apr 11, 2017
src: remove usage of deprecated debug API
42eee8a 
Debug::GetMirror is used in contextify to force loading of the debug
context. There is a better way to achieve this.

PR-URL: nodejs/node#11437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

bnoordhuis bnoordhuis approved these changes
jasnell jasnell approved these changes
addaleax addaleax approved these changes
+2 more reviewers
evanlucas evanlucas approved these changes
mhdawson mhdawson approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. test Issues and PRs related to the tests. v8 engine Issues and PRs related to the V8 dependency. vm Issues and PRs related to the vm subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL