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

crypto: move Decipher.finaltol to End-of-Life #19941

Closed
wants to merge 1 commit into from
Closed

crypto: move Decipher.finaltol to End-of-Life #19941

wants to merge 1 commit into from

Conversation

tniessen
Copy link
Member

Decipher.finaltol was deprecated in #19353 and should be removed in node 11.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@tniessen tniessen added the semver-major PRs that contain breaking changes and should be released in the next major version. label Apr 11, 2018
@nodejs-github-bot nodejs-github-bot added the crypto Issues and PRs related to the crypto subsystem. label Apr 11, 2018
@BridgeAR
Copy link
Member

CI https://ci.nodejs.org/job/node-test-pull-request/14200/

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 11, 2018
@BridgeAR BridgeAR requested a review from a team April 11, 2018 13:25
@BridgeAR BridgeAR added this to the 11.0.0 milestone Apr 11, 2018
@BridgeAR BridgeAR added the fast-track PRs that do not need to wait for 48 hours to land. label Apr 12, 2018
@BridgeAR
Copy link
Member

Suggesting to fast track since three TSC members already gave their LG.

@jasnell
Copy link
Member

jasnell commented Apr 12, 2018

We shouldn't fast-track semver-majors, especially ones that remove APIs. This needs a CITGM run.

@ChALkeR ChALkeR removed the fast-track PRs that do not need to wait for 48 hours to land. label Apr 13, 2018
@ChALkeR
Copy link
Member

ChALkeR commented Apr 13, 2018

@BridgeAR

Suggesting to fast track since three TSC members already gave their LG.

fast-track doesn't work that way. The cases when it could be used are defined in COLLABORATOR_GUIDE, and the number of reviewers isn't a reason for applying fast-track.

This PR could wait for more time to give more people an opportunity to notice it.

@tniessen
Copy link
Member Author

CitGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1364/

@jasnell
Copy link
Member

jasnell commented Apr 14, 2018

@nodejs/tsc ... any objections to this landing in 10.0.0?

jasnell pushed a commit that referenced this pull request Apr 14, 2018
@tniessen @jasnell
crypto: move Decipher.finaltol to End-of-Life
2f97759 
Refs: #19353

PR-URL: #19941
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
@jasnell
Copy link
Member

jasnell commented Apr 14, 2018

Landed in master if 2f97759

@jasnell jasnell closed this Apr 14, 2018
@tniessen
Copy link
Member Author

@jasnell FYI, I believe #19353 is semver-major and targets node 10 as well, so that would be a doc-only deprecation followed by an EOL in a single release.

@ChALkeR
Copy link
Member

ChALkeR commented Apr 15, 2018 •
edited
Loading

@jasnell I reviewed this while it was labelled with a Node.js 11.0.0 milestone.

I am -1 to landing this in 10.0.0 for the reason @tniessen mentioned above — it was runtime-deprecated just in 10.0.0.

@jasnell
Copy link
Member

jasnell commented Apr 15, 2018

Awesome. That's what I suspected. Will make sure it's not included :)

@tniessen tniessen added the deprecations Issues and PRs related to deprecations. label Apr 28, 2018
@targos targos removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trivikr trivikr trivikr approved these changes

@gibfahn gibfahn gibfahn approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

@thefourtheye thefourtheye thefourtheye approved these changes

@jasnell jasnell jasnell approved these changes

@ChALkeR ChALkeR ChALkeR approved these changes

Assignees
No one assigned
Labels
crypto Issues and PRs related to the crypto subsystem. deprecations Issues and PRs related to deprecations. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@tniessen @BridgeAR @jasnell @ChALkeR @thefourtheye @TimothyGu @gibfahn @trivikr @targos @nodejs-github-bot