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

lib, src: use process.config instead of regex #17814

Closed
wants to merge 1 commit into from
Closed

lib, src: use process.config instead of regex #17814

wants to merge 1 commit into from

Conversation

maclover7
Copy link
Contributor

@maclover7 maclover7 commented Dec 21, 2017 •
edited
Loading

Is safer to use a process.binding('config') defined boolean, than to
regex on process.execArgv. Also, this better falls in line with the
conventions of checking flags passed to the executable.

I verified that test/async-hooks/test-callback-error.js properly tests this.

First time contributing to the C++ side of Node, please send all the advice 😆

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

lib, src, async_hooks

@maclover7
lib, src: use process.config instead of regex
e6f3d17 
Is safer to use a `process.binding('config')` defined boolean, than to
regex on `process.execArgv`. Also, this better falls in line with the
conventions of checking flags passed to the executable.
@nodejs-github-bot nodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. labels Dec 21, 2017
@maclover7
Copy link
Contributor Author

maclover7 commented Dec 23, 2017 •
edited
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/12271/, https://ci.nodejs.org/job/node-test-commit-linuxone/11419/

@maclover7
Copy link
Contributor Author

Landing...

@maclover7 maclover7 self-assigned this Dec 24, 2017
@maclover7
Copy link
Contributor Author

Landed in 1dd9c82

@maclover7 maclover7 closed this Dec 24, 2017
@maclover7 maclover7 deleted the jm-abort-config branch December 24, 2017 03:21
maclover7 added a commit to maclover7/node that referenced this pull request Dec 24, 2017
@maclover7
lib, src: use process.config instead of regex
1dd9c82 
Is safer to use a `process.binding(config)` defined boolean, than to
regex on `process.execArgv`. Also, this better falls in line with the
conventions of checking flags passed to the executable.

PR-URL: nodejs#17814
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
@maclover7 @MylesBorins
lib, src: use process.config instead of regex
220894c 
Is safer to use a `process.binding(config)` defined boolean, than to
regex on `process.execArgv`. Also, this better falls in line with the
conventions of checking flags passed to the executable.

PR-URL: #17814
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
@maclover7 @MylesBorins
lib, src: use process.config instead of regex
570f389 
Is safer to use a `process.binding(config)` defined boolean, than to
regex on `process.execArgv`. Also, this better falls in line with the
conventions of checking flags passed to the executable.

PR-URL: #17814
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
@maclover7 @MylesBorins
lib, src: use process.config instead of regex
2d61b9e 
Is safer to use a `process.binding(config)` defined boolean, than to
regex on `process.execArgv`. Also, this better falls in line with the
conventions of checking flags passed to the executable.

PR-URL: #17814
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 10, 2018
Merged
MylesBorins pushed a commit that referenced this pull request May 22, 2018
@maclover7 @MylesBorins
lib, src: use process.config instead of regex
524efb9 
Is safer to use a `process.binding(config)` defined boolean, than to
regex on `process.execArgv`. Also, this better falls in line with the
conventions of checking flags passed to the executable.

PR-URL: #17814
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jun 14, 2018
@maclover7 @MylesBorins
lib, src: use process.config instead of regex
1d6161e 
Is safer to use a `process.binding(config)` defined boolean, than to
regex on `process.execArgv`. Also, this better falls in line with the
conventions of checking flags passed to the executable.

PR-URL: #17814
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jul 9, 2018
Merged
rvagg pushed a commit that referenced this pull request Aug 16, 2018
@maclover7 @rvagg
lib, src: use process.config instead of regex
7c6e391 
Is safer to use a `process.binding(config)` defined boolean, than to
regex on `process.execArgv`. Also, this better falls in line with the
conventions of checking flags passed to the executable.

PR-URL: #17814
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell jasnell approved these changes

Assignees

@maclover7 maclover7

Labels
async_hooks Issues and PRs related to the async hooks subsystem. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@maclover7 @jasnell @cjihrig @MylesBorins @AndreasMadsen @nodejs-github-bot