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.

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: clean up deprecations.md doc. #29725

Closed

Conversation

lholmquist
Copy link
Contributor

This removes the --pending-deprecation label from the Buffer() constructor in the deprecation.md docs.

It was added originally, since it was just a Documentation only deprecation.
It is now a full runtime deprecation, which by default will output what
the flag does anyway, so it can probably be removed

While i was starting to add docs for #29671, i noticed this.

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

@nodejs-github-bot nodejs-github-bot added deprecations Issues and PRs related to deprecations. doc Issues and PRs related to the documentations. labels Sep 26, 2019
* This removes the --pending-deprecation label
  from the Buffer() constructor.  It was added originally,
  since it was just a Documentation only deprecation.
  It is now a full runtime deprecation,
  which by default will output what the  flag does anyway
@lholmquist lholmquist force-pushed the deprecation-readme-cleanup-buffer branch from 1ad7571 to c32edab Compare September 26, 2019 19:40
Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

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

This does not seem right to me. The buffer deprecation is special and it partially prints a runtime warning when the code is coming from outside of the node_modules.
Otherwise it only prints the warning with --pending-deprecation.

@jasnell
Copy link
Member

jasnell commented Sep 26, 2019

I think then that the right thing to do is better clarify the use of --pending-deprecation here.

@Trott
Copy link
Member

Trott commented Sep 30, 2019

I think then that the right thing to do is better clarify the use of --pending-deprecation here.

I opened #29769 as an alternative but feel free to cherry-pick or copy in those changes and modify them to your liking.

@lholmquist
Copy link
Contributor Author

@Trott going to close this in favor of yours, since it address the issue better. I didn't realize there was more to the flag for this particular example.

@Trott Trott closed this Sep 30, 2019
Trott added a commit that referenced this pull request Oct 2, 2019
Refs: #29725 (comment)

PR-URL: #29769
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BridgeAR pushed a commit that referenced this pull request Oct 9, 2019
Refs: #29725 (comment)

PR-URL: #29769
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecations Issues and PRs related to deprecations. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants