-
-
Notifications
You must be signed in to change notification settings - Fork 764
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
Implement type conversions on BigNum #1214
base: master
Are you sure you want to change the base?
Conversation
eb7bb1c
to
fe792c8
Compare
@sfackler ping |
@sfackler bump |
@sfackler We are patching for this downstream. The latest release broke our build. Could you please review this so that we can get it in a release soon? Thanks! |
@sfackler Releasing v0.10.28 broke our build again. Can you please review this PR? |
@sfackler Could you at least comment and let me know the status of this PR? Thanks! |
@sfackler Any update? v0.10.29 broke our build again. |
e9e1637
to
bb87e29
Compare
The Rust-idiomatic TryFrom implementations can be used instead.
@sfackler I've rebased this PR against master. Today marks 8 months since I first submitted this PR; but I haven't heard anything from you. You still seem to be active in pushing releases, so I'm assuming you're around. I'm happy to trade reviews for something that is a high priority for you (either here or elsewhere...) if you'd like. Please let me know anything I can do to help. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Makes conversions a lot more idiomatic, but I think this would need a version bump from 0.10 to 0.11, since it changes the interface quite a bit.
@petreeftime Although it does change the interface, this patch set is fully backwards compatible. A bump from |
I think deprecated functions can break builds for people in some situations, so it might be safer to have a version bump, which would prevent this. I am not an owner of this repo, so ultimately it's not my decision, I just like the patches and wanted to implement something similar. |
The type conversions both closely wrap existing (1.1.0) OpenSSL APIs and are idiomatic Rust.
Deprecate:
BigNum::to_vec()
,BigNum::from_slice()
andBigNum::from_u32()
in favor of the new idiomatic APIs.Also, silence warnings about
std::mem::uninitialized()
on newer Rust.Minimum Required Rust Version is now 1.34 (for
TryFrom
).