Conversation
|
|
||
| #[allow(clippy::large_enum_variant)] | ||
| #[derive(asn1::Asn1Write, asn1::Asn1Read)] | ||
| pub enum CertificateChoices<'a> { |
There was a problem hiding this comment.
I did not mean to suggest we should add these entire enums. My suggestion was to split out the type-alises you'd broken out of the existing code.
(Adding these entire untested enums is the opposite of my goal.)
There was a problem hiding this comment.
Well, CertificateSet contains CertificateChoices. RevocationInfoChoices contains RevocationInfoChoice. So on and so forth, I can't split out anything besides OCSPResponseStatus on its own.
There was a problem hiding this comment.
You can split out the type alias using the current tyeps, e.g.
pub type CertificateSet<'a> = common::Asn1ReadableOrWritable<
asn1::SetOf<'a, certificate::Certificate<'a>>,
asn1::SetOfWriter<'a,certificate::Certificate<'a>, Vec<certificate::Certificate<'a>>>,
>;
There was a problem hiding this comment.
It makes the substantiation of /Certificate/CertificateChoices/ a smaller diff.
It's not the biggest thing in the world, but if you want to make the diff easier to review, it does contribute.
There was a problem hiding this comment.
It won't even become a noticeably smaller diff because second patch has to update these structures to use the new types? :D
2 participants
No description provided.