tls: remove deprecated tls.createSecurePair and SecurePair #57361
+10
−663
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
|
Review requested:
|
nodejs-github-bot
added
needs-ci
anonrig
approved these changes
Mar 7, 2025
JonasBa
force-pushed
the
jb/tls/secure-pair
branch
from
c697c14 to
6f0b0b6
Compare
anonrig
added
semver-major
|
cc @nodejs/tsc since this requires 2 approvals from TSC members. |
JonasBa
force-pushed
the
jb/tls/secure-pair
branch
from
6f0b0b6 to
7774fb3
Compare
JonasBa
commented
Mar 7, 2025
Comment on lines
-584
to
-598
| ### Event: `'secure'` | ||
|
|
||
| <!-- YAML | ||
| added: v0.3.2 | ||
| deprecated: v0.11.3 | ||
| --> | ||
|
|
||
| The `'secure'` event is emitted by the `SecurePair` object once a secure | ||
| connection has been established. | ||
|
|
||
| As with checking for the server | ||
| [`'secureConnection'`][] | ||
| event, `pair.cleartext.authorized` should be inspected to confirm whether the | ||
| certificate used is properly authorized. | ||
|
|
There was a problem hiding this comment.
Given that SecurePair class can now be removed, is it correct to remove this?
|
A github code search reveals a handful of uses in the ecosystem still but nothing that's been updated recently and nothing that's broadly depended on, so yeah, I'd agree it's likely safe. |
jasnell
approved these changes
Mar 7, 2025
JonasBa
force-pushed
the
jb/tls/secure-pair
branch
from
7774fb3 to
ddc6df8
Compare
anonrig
approved these changes
Mar 7, 2025
github-actions
bot
removed
the
request-ci
anonrig
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #57361 +/- ##
=======================================
Coverage 90.20% 90.21%
=======================================
Files 630 629 -1
Lines 185307 185214 -93
Branches 36269 36253 -16
=======================================
- Hits 167162 167093 -69
- Misses 11084 11088 +4
+ Partials 7061 7033 -28
🚀 New features to boost your workflow:
|
@jasnell It had not occurred to me to do that, but I will make sure to check it myself next time, thank you! |
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.
createSecurePair has been deprecated since version 8, it should be safe to remove.
As far as I understand, there is a citgm workflow that would need to be run here to ensure we dont break any popular packages. Would someone be able to run that?
There is also a file left in
benchmark/tls/secure-pair.js, which should probably be renamed now that securePair had been removed.