-
Notifications
You must be signed in to change notification settings - Fork 124
Consider eagerly implementing common traits #340
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
Comments
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'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. |
Feel free to manage/close this PR whenever, by the way. There are no further requests on my end after #341 🙂. |
Uh oh!
There was an error while loading. Please reload this page.
My immediate need/use case is
Clone
onCertificateSigningRequest
. 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 #17 (comment).Prior related work:
Certificate
cloneable (deriveClone
) #319If 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 #316. I'm open to other approaches too.
Copy
,Clone
Debug
Debug
trait #343Display
PartialEq
,Eq
PartialEq
andEq
traits #344PartialOrd
,Ord
PartialOrd
andOrd
traits #346Hash
Hash
trait #345Default
The text was updated successfully, but these errors were encountered: