doc: clearer doc-only deprecations #20381
Closed
Conversation
This file contains 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
Explicitely mention that a documentation only deprecation does not always imply that it will be staged for deprecation in a future Node.js major release. It is mainly there to tell developers that a specific API should be avoided.
COLLABORATOR_GUIDE.md
Outdated
| implemented in the code. There will be no runtime deprecation warnings emitted | ||
| for such deprecations at runtime by default. Documentation-only deprecations | ||
| may trigger a runtime warning when Node.js is started with the | ||
| [`--pending-deprecation`][] flag or the `NODE_PENDING_DEPRECATION=1` |
There was a problem hiding this comment.
Nit: maybe it is worth to also linkify [`NODE_PENDING_DEPRECATION=1`][] and add the bottom reference:
[`NODE_PENDING_DEPRECATION=1`]: doc/api/cli.md#node_pending_deprecation1
vsemozhetbyt
approved these changes
Apr 28, 2018
addaleax
approved these changes
Apr 28, 2018
vsemozhetbyt
added
the
author ready
trivikr
approved these changes
Apr 28, 2018
TimothyGu
approved these changes
Apr 28, 2018
Trott
reviewed
Apr 28, 2018
COLLABORATOR_GUIDE.md
Outdated
| is started with the [`--pending-deprecation`][] flag or the | ||
| `NODE_PENDING_DEPRECATION=1` environment variable is set. | ||
| * *Documentation-Only Deprecation* refers to elements of the Public API that | ||
| should be avoided by developers and that might be staged for deprecation in a |
There was a problem hiding this comment.
The word "deprecation" does not mean removal. It means discouragement of use. So "staged for deprecation" makes no sense in this context. A documentation deprecation is a deprecation. I think you might mean "staged for removal"? Although I would just go with "removed":
should be avoided by developers and that might be removed in a
There was a problem hiding this comment.
I see that "staged for deprecation" was existing text, not new text introduced here. I'd still encourage removing it as it is ambiguous at best. "Removed" is clear. Or if you mean a runtime warning, then say that. Either way, more precise and explicit is better.
There was a problem hiding this comment.
(Above comments are suggestions, but not blocking objections.)
Trott
reviewed
Apr 28, 2018
COLLABORATOR_GUIDE.md
Outdated
| future Node.js major release. An explicit notice indicating the deprecation | ||
| status is added to the API documentation but no functional changes are | ||
| implemented in the code. There will be no runtime deprecation warnings emitted | ||
| for such deprecations at runtime by default. Documentation-only deprecations |
There was a problem hiding this comment.
Please remove either the first or the last use of "runtime" in "runtime deprecation warnings emitted for such deprecations at runtime".
|
@Trott comments addressed. PTAL |
|
I am going to land this in ~ 24h if there are no objections. |
jasnell
approved these changes
May 6, 2018
BridgeAR
added a commit
to BridgeAR/node
that referenced
this pull request
May 7, 2018
Explicitely mention that a documentation only deprecation does not always imply that it will be staged for deprecation in a future Node.js major release. It is mainly there to tell developers that a specific API should be avoided. PR-URL: nodejs#20381 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in 28a54cb |
MylesBorins
pushed a commit
that referenced
this pull request
May 8, 2018
Explicitely mention that a documentation only deprecation does not always imply that it will be staged for deprecation in a future Node.js major release. It is mainly there to tell developers that a specific API should be avoided. PR-URL: #20381 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Merged
MylesBorins
pushed a commit
that referenced
this pull request
May 8, 2018
Explicitely mention that a documentation only deprecation does not always imply that it will be staged for deprecation in a future Node.js major release. It is mainly there to tell developers that a specific API should be avoided. PR-URL: #20381 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
May 9, 2018
Explicitely mention that a documentation only deprecation does not always imply that it will be staged for deprecation in a future Node.js major release. It is mainly there to tell developers that a specific API should be avoided. PR-URL: #20381 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explicitely mention that a documentation only deprecation does not
always imply that it will be staged for deprecation in a future
Node.js major release. It is mainly there to tell developers that
a specific API should be avoided.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes