Consider eagerly implementing common traits
My immediate need/use case is Clone on CertificateSigningRequest. After skimming the codebase, however, I'm getting the impression that there are many more opportunities to implement the common traits outlined in the API guidelines. @est31 also echoed this over at https://github.com/rustls/rcgen/issues/17#issuecomment-503656409.
Prior related work:
- https://github.com/rustls/rcgen/issues/315 / https://github.com/rustls/rcgen/pull/316
- https://github.com/rustls/rcgen/pull/319
- and more, probably...
If you're amenable to eagerly implementing these traits, I think it'd be easiest (from a review standpoint) to create a pull request per trait and dedicate each PR to sweeping the codebase and nitpicking the details—similar to https://github.com/rustls/rcgen/pull/316. I'm open to other approaches too.
- [x]
Copy,Clone- [x] https://github.com/rustls/rcgen/pull/341
- [ ]
Debug- [x] https://github.com/rustls/rcgen/pull/316
- [ ] https://github.com/rustls/rcgen/pull/343
- [x]
Display- [x] (Deemed not necessary for now)
- [ ]
PartialEq,Eq- [ ] https://github.com/rustls/rcgen/pull/344
- [x]
PartialOrd,Ord- [x] (Closed) https://github.com/rustls/rcgen/pull/346
- [x]
Hash- [x] (Closed) https://github.com/rustls/rcgen/pull/345
- [x]
Default- [x] (Deemed not necessary for now)
After skimming the codebase, however, I'm getting the impression that there are many more opportunities to implement the common traits outlined in the API guidelines.
Agreed! I think some of these were harder to take back when some of the params types were holding private data but we're in a better spot now.
I think it'd be easiest (from a review standpoint) to create a pull request per trait and dedicate each PR to sweeping the codebase and nitpicking the details
I'd also be open to one PR with each trait handled per-commit but don't feel strongly.
One trait per commit in a single PR sounds good to me.
#341 was pretty seamless, so I'll open one more PR for all the other common traits—one trait per commit.
I think we should stop after the PRs that already exist and wait for someone who has a particular impl they require.
I think we should stop after the PRs that already exist
Feel free to manage/close this PR whenever, by the way. There are no further requests on my end after https://github.com/rustls/rcgen/pull/341 🙂.
@lvkv I think you check off all items on the list and close this issue now as all PRs related to your list have been merged into main or closed with a reason.
@Rynibami @lvkv Thanks to both of you for your work here. I'll close this as completed.