crypto: set DEFAULT_ENCODING property to non-enumerable by aduh95 · Pull Request #23222 · nodejs/node (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation5 Commits1 Checks0 Files changed

Conversation

This file contains hidden or 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

[ Show hidden characters]({{ revealButtonHref }})

aduh95

Since it is a deprecated API, a deprecation warning is printed when
loading crypto module from ESM. Making it non enumerable removes the
deprecation warning and makes the API non-available to named imports.

Fixes: #23203

The Credentials API was already removed by #21153.

It is a breaking change, but IMHO it should be integrated to Node 11 before it's released as it is a minor semver change.

Checklist

@aduh95

Since it is a deprecated API, a deprecation warning is printed when loading crypto module from ESM. Making it non enumerable remove the deprecation warning and make the API non-available to named imports.

Fixes: nodejs#23203

@Trott Trott added the semver-major

PRs that contain breaking changes and should be released in the next major version.

label

Oct 2, 2018

@Trott Trott added this to the 11.0.0 milestone

Oct 2, 2018

cjihrig

guybedford

jasnell

addaleax

thefourtheye

@guybedford

refack

@jasnell

Taking this off the 11.0.0 milestone. While it appears to have the necessary approvals to land, we're less than a week away from the 11.0.0 release and well past the semver-major cut off. There doesn't appear to be anything critical about this change so it's going to get pushed out.

@jasnell jasnell removed this from the 11.0.0 milestone

Oct 17, 2018

tniessen

@guybedford

guybedford pushed a commit that referenced this pull request

Nov 6, 2018

@aduh95 @guybedford

Since it is a deprecated API, a deprecation warning is printed when loading crypto module from ESM. Making it non enumerable remove the deprecation warning and make the API non-available to named imports.

PR-URL: #23222 Fixes: #23203 Reviewed-By: Colin Ihrig cjihrig@gmail.com Reviewed-By: Guy Bedford guybedford@gmail.com Reviewed-By: James M Snell jasnell@gmail.com Reviewed-By: Anna Henningsen anna@addaleax.net Reviewed-By: Sakthipriyan Vairamani thechargingvolcano@gmail.com Reviewed-By: Refael Ackermann refack@gmail.com Reviewed-By: Tobias Nießen tniessen@tnie.de

kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request

Nov 15, 2018

@aduh95 @kiyomizumia

Since it is a deprecated API, a deprecation warning is printed when loading crypto module from ESM. Making it non enumerable remove the deprecation warning and make the API non-available to named imports.

PR-URL: nodejs#23222 Fixes: nodejs#23203 Reviewed-By: Colin Ihrig cjihrig@gmail.com Reviewed-By: Guy Bedford guybedford@gmail.com Reviewed-By: James M Snell jasnell@gmail.com Reviewed-By: Anna Henningsen anna@addaleax.net Reviewed-By: Sakthipriyan Vairamani thechargingvolcano@gmail.com Reviewed-By: Refael Ackermann refack@gmail.com Reviewed-By: Tobias Nießen tniessen@tnie.de

@alanosman

Hi folks - I just installed 11.6 and I still see this warning. When does this change make it into stable? Also how do I easily figure out what release this made it into without asking? thx

@cjihrig

This was removed from the 11.0.0 milestone, and because it is semver major, that means it won't go out in a release until 12.0.0.

BethGriggs added a commit that referenced this pull request

Apr 22, 2019

@BethGriggs

Notable changes:

PR-URL: #26930

BethGriggs added a commit that referenced this pull request

Apr 23, 2019

@BethGriggs

Notable changes:

PR-URL: #26930

BethGriggs added a commit that referenced this pull request

Apr 23, 2019

@BethGriggs

Notable changes:

PR-URL: #26930

This was referenced

Apr 23, 2019

This was referenced

Apr 23, 2019

Reviewers

@refack refack refack approved these changes

@jasnell jasnell jasnell approved these changes

@guybedford guybedford guybedford approved these changes

@thefourtheye thefourtheye thefourtheye approved these changes

@addaleax addaleax addaleax approved these changes

@cjihrig cjihrig cjihrig approved these changes

@tniessen tniessen tniessen approved these changes

Labels

crypto

Issues and PRs related to the crypto subsystem.

semver-major

PRs that contain breaking changes and should be released in the next major version.