-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CRLs to rustls feature #2433
Conversation
059c542
to
03ee65a
Compare
03ee65a
to
b8e7562
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR! I think this'll be a nice addition. I did leave some comments inline, some of which I think should be changed, and others I'm open to hearing the rationale. 🚀
src/lib.rs
Outdated
@@ -349,6 +349,8 @@ if_hyper! { | |||
#[cfg(feature = "__tls")] | |||
// Re-exports, to be removed in a future release | |||
pub use tls::{Certificate, Identity}; | |||
#[cfg(feature = "__rustls")] | |||
pub use tls::Crl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the other re-exports will be removed from the top-level eventually, we probably shouldn't add this one. People should reach for it from reqwest::tls::...
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. Thank you. I will fix this.
@seanmonstar thank you very much for the review! I hope that this PR will be useful for others. I replied to your comments. |
@seanmonstar could you, please, review PR after suggested changes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, thank you!
No description provided.