Skip to content
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

Replace Deref impl on RsaPrivateKey with AsRef #317

Merged
merged 1 commit into from
Apr 27, 2023

Conversation

tarcieri
Copy link
Member

The RsaPrivateKey type previously had a Deref impl providing access to the associated RsaPublicKey.

Deref is intended for "smart pointer types", i.e. container types which manage a (typically generic) inner type in some way. This doesn't seem like one of those cases.

AsRef, on the other hand, is for cheap reference conversions, which is exactly what's happening here, so it's a better fit and provides the same functionality (albeit explicitly rather than via deref coercion).

The `RsaPrivateKey` type previously had a `Deref` impl providing access
to the associated `RsaPublicKey`.

`Deref` is intended for "smart pointer types", i.e. container types
which manage a (typically generic) inner type in some way. This doesn't
seem like one of those cases.

`AsRef`, on the other hand, is for cheap reference conversions, which is
exactly what's happening here, so it's a better fit and provides the
same functionality (albeit explicitly rather than via deref coercion).
@tarcieri tarcieri force-pushed the replace-pubkey-deref-with-asref branch from 05b1846 to 390ad37 Compare April 27, 2023 14:35
@tarcieri tarcieri requested a review from dignifiedquire April 27, 2023 14:40
@tarcieri
Copy link
Member Author

FYI: this is the last change I have in mind prior to a v0.9.0 release

cc @lumag

@tarcieri tarcieri merged commit ec49956 into master Apr 27, 2023
@tarcieri tarcieri deleted the replace-pubkey-deref-with-asref branch April 27, 2023 15:24
@lumag
Copy link
Contributor

lumag commented Apr 27, 2023

@tarcieri please excuse me for missing several last PRs. I am at the conference now. They look good for me.

@tarcieri tarcieri mentioned this pull request Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants