-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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.
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
doc: add code example to process.throwDeprecation property #29495
Conversation
true | ||
$ node -p "process.throwDeprecation" | ||
undefined | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be worthwhile to include an example that shows process.throwDeprecation
is mutable at runtime...
process.throwDeprecation = true;
process.emitWarning('test', 'DeprecationWarning');
// will cause an error to be thrown
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally worth it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should docs change? I mean, may say that process. throwDeprecation
is mutable at runtime like: "The process.throwDeprecation
is a mutable property at runtime which indicates...". And of course, add your example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be good, I think
doc/api/process.md
Outdated
documentation for the [`'warning'` event][process_warning] and the | ||
[`emitWarning()` method][process_emit_warning] for more information about this | ||
flag's behavior. | ||
The `process.throwDeprecation` is a mutable property at runtime which |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This text is a little confusing.
- It's not clear what the difference between "mutable property at runtime" vs. "mutable property" would be. I don't think there is any in this case.
- Since it is mutable, it doesn't necessarily indicate whether the command line flag is set.
Maybe something more like this?
The initial value of `process.throwDeprecation` indicates whether the
`--throw-deprecation` flag is set on the current Node.js process. See
the documentation for the [`'warning'` event][process_warning] and the
[`emitWarning()` method][process_emit_warning] for more information.
`process.throwDeprecation` is mutable, so whether or not deprecation
warnings results in an error may be altered at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeap, that's quite confusing.
Your proposal seems to be more clear than mine (on my opinion), but on my reading experience, I think is quite better switch the order of the sentences about process.throwDeprecation
property and emitWarning()
and warning
event, to make that paragraph with a little bit more of cohesion.
Fixed on bf3bcf7.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see the text edited a bit more before landing.
doc/api/process.md
Outdated
The `process.throwDeprecation` property indicates whether the | ||
`--throw-deprecation` flag is set on the current Node.js process. See the | ||
The initial value of `process.throwDeprecation` indicates whether the | ||
`--throw-deprecation` flag is set on the current Node.js process, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`--throw-deprecation` flag is set on the current Node.js process, | |
`--throw-deprecation` flag is set on the current Node.js process. |
doc/api/process.md
Outdated
The initial value of `process.throwDeprecation` indicates whether the | ||
`--throw-deprecation` flag is set on the current Node.js process, | ||
`process.throwDeprecation` is mutable, so whether or not deprecation | ||
warnings results in an error may be altered at runtime. See the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warnings results in an error may be altered at runtime. See the | |
warnings result in errors may be altered at runtime. See the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR-URL: #29495 Reviewed-By: David Carlier <[email protected]> Reviewed-By: James M Snell <[email protected]>
Landed in 68d6c4c |
PR-URL: #29495 Reviewed-By: David Carlier <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #29495 Reviewed-By: David Carlier <[email protected]> Reviewed-By: James M Snell <[email protected]>
Checklist