Split certificate module out of crate root #237
Conversation
I think this is better, let's have them all in one place, then one knows where to look it up. |
rcgen/src/certificate.rs
Outdated
| write_distinguished_name, write_dt_utc_or_generalized, write_x509_authority_key_identifier, | ||
| write_x509_extension, DistinguishedName, Error, KeyIdMethod, KeyPair, KeyUsagePurpose, SanType, | ||
| SerialNumber, OID_BASIC_CONSTRAINTS, OID_COMMON_NAME, OID_COUNTRY_NAME, | ||
| OID_CRL_DISTRIBUTION_POINTS, OID_EXT_KEY_USAGE, OID_KEY_USAGE, OID_LOCALITY_NAME, |
There was a problem hiding this comment.
Imports with these many lines are an anti-pattern as their interaction with rustfmt and text reflow is prone to cause merge conflicts. If you removed the OID_ prefix from the oids and moved all the OID_ related stuff to a separate mod oid, contained inside lib.rs, and then used the OIDs e.g. via oid::BASIC_CONSTRAINTS, then the number of lines of the import would be greatly reduced.
Alternatively, one could have a star import use crate::oid::*. I generally dislike star imports but if everything in the module is prefixed with the OID_ name then it's pretty clear where the item is from.
There was a problem hiding this comment.
Good call! There was actually already an oid module, I just failed to notice it. Imported the module only in the new certificate module to reduce the number of imports here.
|
Thank you! I've also found |
Before this PR,
lib.rswas 2258 lines long, and I found it quite hard to navigate. Here, I tried to split off acertificatemodule coming in at 1497 lines (1215 without tests), reducinglib.rsto 796 lines.The new module also follows a top-down ordering, following the prevailing style in rustls projects. While some of the smaller modules seem to be top-down oriented,
lib.rsso far was closer to bottom-up, but its lack of consistent ordering was definitely one of the reasons I've found it hard to navigate.To build this, I took
CertificateandCertificateParamsand move them into a separate module. Any types that are mainly used viaCertificateandCertificateParamsare also moved. I have kept the OID definitions together (rather than moving the certificate-related ones tocertificate) -- maybe we should move the ones that are only used incertificate?At least for me, this will be much easier to navigate.