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: fix error in deprecation message #6344

Closed
wants to merge 1 commit into from
Closed

crypto: fix error in deprecation message #6344

wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Apr 22, 2016

Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

crypto test

Description of change

The deprecation message for crypto.Credentials says to use
tls.createSecureContext but the correct property to use is
tls.SecureContext().

Fix the deprecation message and add a test that checks the mappings of
deprecated properties and their warning messages.

@nodejs/crypto

@Trott Trott added crypto Issues and PRs related to the crypto subsystem. test Issues and PRs related to the tests. labels Apr 22, 2016
@jasnell
Copy link
Member

jasnell commented Apr 22, 2016

LGTM

@Trott
crypto: fix error in deprecation message
6e019f5 
The deprecation message for `crypto.Credentials` says to use
`tls.createSecureContext` but the correct property to use is
`tls.SecureContext()`.

Fix the deprecation message and add a test that checks the mappings of
deprecated properties and their warning messages.
@Trott
Copy link
Member Author

Trott commented Apr 27, 2016

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

@Trott
Copy link
Member Author

Trott commented Apr 28, 2016

CI failures are one buildbot failure and the current problematic tick processor issue on OS X. Unrelated, of course.

Trott added a commit to Trott/io.js that referenced this pull request Apr 28, 2016
@Trott
crypto: fix error in deprecation message
296bfd2 
The deprecation message for `crypto.Credentials` says to use
`tls.createSecureContext` but the correct property to use is
`tls.SecureContext()`.

Fix the deprecation message and add a test that checks the mappings of
deprecated properties and their warning messages.

PR-URL: nodejs#6344
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Copy link
Member Author

Trott commented Apr 28, 2016

Landed in 296bfd2

@Trott Trott closed this Apr 28, 2016
Fishrock123 pushed a commit that referenced this pull request May 4, 2016
@Trott @Fishrock123
crypto: fix error in deprecation message
fdde369 
The deprecation message for `crypto.Credentials` says to use
`tls.createSecureContext` but the correct property to use is
`tls.SecureContext()`.

Fix the deprecation message and add a test that checks the mappings of
deprecated properties and their warning messages.

PR-URL: #6344
Reviewed-By: James M Snell <jasnell@gmail.com>
This was referenced May 4, 2016
Closed
Merged
@MylesBorins
Copy link
Contributor

@Trott lts?

@Trott
Copy link
Member Author

Trott commented Jun 1, 2016

@thealphanerd Yes.

@MylesBorins
Copy link
Contributor

MylesBorins commented Jun 1, 2016 •
edited
Loading

@Trott test failure on v4.x-staging, can you manually backport?

edit: please include all original meta data

@Trott
Copy link
Member Author

Trott commented Jun 2, 2016

@thealphanerd Is the test failing because warning events don't exist in v4 or something like that? (Link to failing test?)

@MylesBorins
Copy link
Contributor 

Path: parallel/test-crypto-deprecated
Mismatched <anonymous> function calls. Expected 2, actual 0.
    at Object.<anonymous> (/Users/thealphanerd/code/node/v4.x/test/parallel/test-crypto-deprecated.js:12:30)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)
    at startup (node.js:139:18)
    at node.js:968:3
(node) crypto.Credentials is deprecated. Use tls.SecureContext instead.
(node) crypto.createCredentials is deprecated. Use tls.createSecureContext instead.

@Trott
Copy link
Member Author

Trott commented Jun 2, 2016

This depends on process.emitWarning() which will not land in LTS.

So the test needs to be re-written for LTS.

Or we could choose not to land.

Or you could just land the deprecation message text change and not land the test.

@MylesBorins
Copy link
Contributor

@Trott I'm leaving this as don't land for now. Please feel free to open a new PR

@Trott Trott deleted the deprecrypto branch January 13, 2022 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
crypto Issues and PRs related to the crypto subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Trott @jasnell @MylesBorins