doc: add documentation for deprecation properties #16539

maclover7 · 2017-10-26T23:58:49Z

Relevant commit history:

cc @vsemozhetbyt, I tried to avoid duplicating tons of documentation by linking out to other sections of the process docs, would love your thoughts :)

Checklist

documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

doc

refack · 2017-10-27T00:13:30Z

Code LGTM, but the question is should they be documented or removed. AFAICT they were exposed as a rough mechanism of communicating the CLI flags to JS, they are not necessarily part of a succinct API.

Alternatively they (and other CLI flag mappings) could go behind a new process.cliArgs() API endpoint.

vsemozhetbyt · 2017-10-27T00:09:52Z

doc/api/process.md

+
+* {boolean}
+
+The `process.throwDeprecation` property returns if the `--trace-deprecation`


A nit: --trace-deprecation -> --throw-deprecation

vsemozhetbyt · 2017-10-27T00:13:33Z

doc/api/process.md

@@ -1667,6 +1680,32 @@ process.stdin.pipe(process.stdout);
 *Note*: `process.stdout` differs from other Node.js streams in important ways,
 see [note on process I/O][] for more information.

+## process.traceDeprecation


It seems this should be placed after the process.title, ABC-wise.

my bad -- don't know why I thought a ### about IO was a ## 😞

vsemozhetbyt · 2017-10-27T00:28:26Z

I am not sure about some things.

Can we say that the type is boolean if the value is undefined in case of the CLI key is not set?

console.log(
  process.noDeprecation, process.throwDeprecation, process.traceDeprecation
);

undefined undefined undefined

Should we note that these properties are not just a mirrors of the CLI keys, but they can affect the warning behavior as per util.md notes.
Should we duplicate the precedence note?

Let's see what other collaborators think.

vsemozhetbyt · 2017-10-27T00:36:16Z

@refack It seems we cannot just remove them as they have been documented in util doc some years. We can deprecate them, but we may still need to document them for that in the proper place.

vsemozhetbyt · 2017-10-27T00:40:08Z

BTW, thank you for the PR and digging into the history!

addaleax · 2017-10-27T01:01:28Z

Alternatively they (and other CLI flag mappings) could go behind a new process.cliArgs() API endpoint.

Like process.execArgv? ;)

refack · 2017-10-27T02:18:44Z

Alternatively they (and other CLI flag mappings) could go behind a new process.cliArgs() API endpoint.
Like process.execArgv? ;)

Hence my ambivalence about fully embracing those flags as 1st class API citizens 😕
Although process.execArgv still requires parsing and does not take into account NODE_OPTIONS

BridgeAR · 2017-11-23T21:23:10Z

I personally think it would be best to deprecate these instead of documenting them. At the same time we can rephrase the util part where they are mentioned.

maclover7 · 2017-11-28T00:46:33Z

@BridgeAR Even if the props are deprecated publicly, they are still used within parts of Node core (like lib/internal/process/warning.js for example), so I'm not sold on what the maintenance benefit would be for doing that :(

BridgeAR · 2017-12-07T16:59:18Z

@maclover7 even if we use some internally it does not mean users should always use those things as well. That is what internal is all about. At least that is how I see it. So it is not about a maintenance benefit but about being able to change internal details and or not overloading the user with information that does not improve anything.

nylen · 2017-12-26T20:54:22Z

Please document and standardize these properties. I want to use process.throwDeprecation = true; in my code so that I don't have to start node with --throw-deprecation.

Edit: See also #17871

cjihrig · 2017-12-26T22:02:10Z

doc/api/process.md

+
+* {boolean}
+
+The `process.noDeprecation` property returns if the `--no-deprecation`


Possibly change "returns if" to "indicates whether or not"

I would go with just "indicates whether", without the "or not".

cjihrig · 2017-12-26T22:04:22Z

doc/api/process.md

+
+* {boolean}
+
+The `process.throwDeprecation` property returns if the `--throw-deprecation`


Same comment here and below.

maclover7 · 2017-12-31T18:01:48Z

updated @cjihrig @apapirovski

maclover7 · 2018-01-02T22:05:06Z

ping @cjihrig @apapirovski would like to try and land this

cjihrig

The changes themselves look fine, but there were a few comments by others that brought up the alternative of deprecating and removing these properties.

maclover7 · 2018-01-02T22:22:12Z

CI: https://ci.nodejs.org/job/node-test-commit-light/88/

maclover7 · 2018-01-02T22:22:32Z

Going to open a followup issue to discuss possible deprecation/removal of the properties.

maclover7 · 2018-01-13T17:09:31Z

c770f43

PR-URL: #16539 Fixes: #16394 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>

nodejs-github-bot added doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem. labels Oct 26, 2017

vsemozhetbyt reviewed Oct 27, 2017

View reviewed changes

jasnell approved these changes Oct 27, 2017

View reviewed changes

MylesBorins force-pushed the master branch from b7405ab to 7f086dd Compare December 8, 2017 16:37

cjihrig reviewed Dec 26, 2017

View reviewed changes

doc: add documentation for deprecation properties

ab6eabe

maclover7 force-pushed the jm-doc-process branch from a1b99a1 to ab6eabe Compare December 31, 2017 18:01

jasnell approved these changes Dec 31, 2017

View reviewed changes

apapirovski approved these changes Jan 2, 2018

View reviewed changes

cjihrig approved these changes Jan 2, 2018

View reviewed changes

maclover7 closed this Jan 13, 2018

maclover7 deleted the jm-doc-process branch January 13, 2018 17:09

evanlucas mentioned this pull request Jan 30, 2018

v9.5.0 release proposal #18464

Merged

MylesBorins added land-on-v6.x labels Feb 27, 2018

This was referenced Feb 27, 2018

v6.13.1 proposal #19025

Closed

v6.13.1 proposal #19027

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: add documentation for deprecation properties #16539

doc: add documentation for deprecation properties #16539

maclover7 commented Oct 26, 2017

refack commented Oct 27, 2017

vsemozhetbyt Oct 27, 2017

vsemozhetbyt Oct 27, 2017

maclover7 Oct 27, 2017

vsemozhetbyt commented Oct 27, 2017 •

edited

Loading

vsemozhetbyt commented Oct 27, 2017

vsemozhetbyt commented Oct 27, 2017

addaleax commented Oct 27, 2017

refack commented Oct 27, 2017

BridgeAR commented Nov 23, 2017

maclover7 commented Nov 28, 2017

BridgeAR commented Dec 7, 2017

nylen commented Dec 26, 2017 •

edited

Loading

cjihrig Dec 26, 2017

apapirovski Dec 27, 2017

cjihrig Dec 26, 2017

maclover7 commented Dec 31, 2017

maclover7 commented Jan 2, 2018

cjihrig left a comment

maclover7 commented Jan 2, 2018

maclover7 commented Jan 2, 2018

maclover7 commented Jan 13, 2018


		* {boolean}

		The `process.throwDeprecation` property returns if the `--trace-deprecation`


		* {boolean}

		The `process.noDeprecation` property returns if the `--no-deprecation`

doc: add documentation for deprecation properties #16539

doc: add documentation for deprecation properties #16539

Conversation

maclover7 commented Oct 26, 2017

Checklist

Affected core subsystem(s)

refack commented Oct 27, 2017

vsemozhetbyt Oct 27, 2017

Choose a reason for hiding this comment

vsemozhetbyt Oct 27, 2017

Choose a reason for hiding this comment

maclover7 Oct 27, 2017

Choose a reason for hiding this comment

vsemozhetbyt commented Oct 27, 2017 • edited Loading

vsemozhetbyt commented Oct 27, 2017

vsemozhetbyt commented Oct 27, 2017

addaleax commented Oct 27, 2017

refack commented Oct 27, 2017

BridgeAR commented Nov 23, 2017

maclover7 commented Nov 28, 2017

BridgeAR commented Dec 7, 2017

nylen commented Dec 26, 2017 • edited Loading

cjihrig Dec 26, 2017

Choose a reason for hiding this comment

apapirovski Dec 27, 2017

Choose a reason for hiding this comment

cjihrig Dec 26, 2017

Choose a reason for hiding this comment

maclover7 commented Dec 31, 2017

maclover7 commented Jan 2, 2018

cjihrig left a comment

Choose a reason for hiding this comment

maclover7 commented Jan 2, 2018

maclover7 commented Jan 2, 2018

maclover7 commented Jan 13, 2018

vsemozhetbyt commented Oct 27, 2017 •

edited

Loading

nylen commented Dec 26, 2017 •

edited

Loading