Skip to content

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Mar 24, 2023

These subvariants were incidentally making enums from internals part of the public API:

  • CertificateStatusType
  • KeyUpdateRequest
  • ECCurveType

But these enums don't add much information. For instance, CertificateStatusType only has one valid value (OCSP), and all other values would be Unknown(u8). Reporting that level of detail for the unexpected response probably belongs more in a byte-by-byte protocol debugger than the workaday Error type.

Also, rename UnsupportedCurve to UnsupportedCurveType to be slightly more accurate.

Part of #920

These subvariants were incidentally making enums from `internals` part
of the public API:

 - CertificateStatusType
 - KeyUpdateRequest
 - ECCurveType

But these enums don't add much information. For instance,
CertificateStatusType only has one valid value (OCSP), and all other
values would be Unknown(u8). Reporting that level of detail for the
unexpected response probably belongs more in a byte-by-byte protocol
debugger than the workaday Error type.

Also, rename UnsupportedCurve to UnsupportedCurveType to be slightly
more accurate.
@codecov
Copy link

codecov bot commented Mar 24, 2023

Codecov Report

Merging #1243 (b042ce0) into main (2b3b797) will not change coverage.
The diff coverage is 33.33%.

@@           Coverage Diff           @@
##             main    #1243   +/-   ##
=======================================
  Coverage   95.89%   95.89%           
=======================================
  Files          61       61           
  Lines       14216    14216           
=======================================
  Hits        13633    13633           
  Misses        583      583           
Impacted Files Coverage Δ
rustls/src/error.rs 95.41% <ø> (ø)
rustls/src/msgs/handshake.rs 98.26% <0.00%> (ø)
rustls/src/common_state.rs 95.01% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Makes sense.

@djc djc merged commit 78b16d4 into rustls:main Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants