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

test: use process.features.debug in common module #16537

Closed
wants to merge 1 commit into from
Closed

test: use process.features.debug in common module #16537

wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Oct 26, 2017

Replace process.config.target_defaults.default_configuration check with
process.features.debug.

Refs: #4431 (comment)

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

test

@Trott
test: use process.features.debug in common module
745e00b 
Replace process.config.target_defaults.default_configuration check with
process.features.debug.

Refs: nodejs#4431 (comment)
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 26, 2017
@Trott
Copy link
Member Author

Trott commented Oct 26, 2017

ping @skomski

jasnell
jasnell reviewed Oct 27, 2017
View reviewed changes
test/common/index.js
@@ -340,7 +340,7 @@ exports.spawnSyncPwd = function(options) {
};

exports.platformTimeout = function(ms) {
if (process.config.target_defaults.default_configuration === 'Debug')
if (process.features.debug)
Copy link
Member

Choose a reason for hiding this comment

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

just keep in mind that process.features is mutable by end users...

@joyeecheung
Copy link
Member

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

@Trott
Copy link
Member Author

Trott commented Oct 27, 2017

Lone CI failure is build related.

@Trott
Copy link
Member Author

Trott commented Oct 27, 2017

Landed in 98eab4a

@Trott Trott closed this Oct 27, 2017
Trott added a commit to Trott/io.js that referenced this pull request Oct 27, 2017
@Trott
test: use process.features.debug in common module
98eab4a 
Replace process.config.target_defaults.default_configuration check with
process.features.debug.

PR-URL: nodejs#16537
Ref: nodejs#4431 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Fishrock123
Copy link
Contributor

When was process.features added? 🤔 Why isn't it documented?

@jasnell
Copy link
Member

jasnell commented Oct 27, 2017

git blame says six years ago: aa0308d

@jasnell
Copy link
Member

jasnell commented Oct 27, 2017

(there's a certain irony in process.features not being documented)

@jasnell jasnell mentioned this pull request Oct 30, 2017
Merged
gibfahn pushed a commit that referenced this pull request Oct 30, 2017
@Trott @gibfahn
test: use process.features.debug in common module
4c737e6 
Replace process.config.target_defaults.default_configuration check with
process.features.debug.

PR-URL: #16537
Ref: #4431 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
gibfahn pushed a commit that referenced this pull request Oct 30, 2017
@Trott @gibfahn
test: use process.features.debug in common module
0121d59 
Replace process.config.target_defaults.default_configuration check with
process.features.debug.

PR-URL: #16537
Ref: #4431 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
gibfahn pushed a commit that referenced this pull request Oct 31, 2017
@Trott @gibfahn
test: use process.features.debug in common module
ea32d0e 
Replace process.config.target_defaults.default_configuration check with
process.features.debug.

PR-URL: #16537
Ref: #4431 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@gibfahn gibfahn mentioned this pull request Oct 31, 2017
Merged
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
@Trott @Qard
test: use process.features.debug in common module
95e4120 
Replace process.config.target_defaults.default_configuration check with
process.features.debug.

PR-URL: nodejs/node#16537
Ref: nodejs/node#4431 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
@Trott @Qard
test: use process.features.debug in common module
8cc7b17 
Replace process.config.target_defaults.default_configuration check with
process.features.debug.

PR-URL: nodejs/node#16537
Ref: nodejs/node#4431 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
@Trott @MylesBorins
test: use process.features.debug in common module
810a89c 
Replace process.config.target_defaults.default_configuration check with
process.features.debug.

PR-URL: #16537
Ref: #4431 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
@Trott @MylesBorins
test: use process.features.debug in common module
3f60021 
Replace process.config.target_defaults.default_configuration check with
process.features.debug.

PR-URL: #16537
Ref: #4431 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
@Trott @MylesBorins
test: use process.features.debug in common module
8663b05 
Replace process.config.target_defaults.default_configuration check with
process.features.debug.

PR-URL: #16537
Ref: #4431 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
@Trott @addaleax
test: use process.features.debug in common module
b83a9d4 
Replace process.config.target_defaults.default_configuration check with
process.features.debug.

PR-URL: nodejs/node#16537
Ref: nodejs/node#4431 (comment)
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott Trott deleted the use-process-features branch January 13, 2022 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack refack approved these changes

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@cjihrig cjihrig cjihrig approved these changes

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

Assignees
No one assigned
Labels
test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@Trott @joyeecheung @Fishrock123 @jasnell @refack @lpinca @cjihrig @gireeshpunathil @MylesBorins @nodejs-github-bot