Enable bitcoin_hashes v0.14.0 #76
Conversation
|
cc @afilini |
|
ACK, that would be helpful for me, all the other libs like rust-bitcoin and bdk have already upgraded to hashes 0.14 |
Cargo.toml
Outdated
| # Enabling the "rand" feature by default to run the benches | ||
| bip39 = { path = ".", features = ["rand"] } | ||
| bitcoin_hashes = ">=0.12,<0.14" # enable default features for test | ||
| bitcoin_hashes = ">=0.12,<=0.14" # enable default features for test |
There was a problem hiding this comment.
This must be >= 0.12, <0.15, otherwise 0.14.1 will be unusable even though it'd be compatible.
There was a problem hiding this comment.
Oh wow, TIL. Will fix, thanks.
There was a problem hiding this comment.
I'm going to go right ahead and 'trust don't verify'.
There was a problem hiding this comment.
The verification is easy, just evaluate 0 < 1 expression in your head. :)
There was a problem hiding this comment.
Oh I thought we were talking about 0.14 working for 0.14.0 but not for 0.14.1.
There was a problem hiding this comment.
Ok, I have no idea what you were talking about in your original comment then. <= 14 is equivalent to < 15
There was a problem hiding this comment.
It's not. 0.14 == 0.14.0
0.14.1 > 0.14.0
0.14.1 < 0.15.0
And the above implies 0.14.1 > 0.14 which is a negation of 0.14.1 <= 0.14
So for <= 14 to be equivalent to < 15 we require all x to satisfy x <= 0.14 <-> x <0.15 and we have a counter example for x = 0.14.1. QED
There was a problem hiding this comment.
I get it now. Seems the confusion started when I wrote "I'm going to go right ahead and 'trust don't verify'." and I have no clue what I was thinking then. Your original review comment is actually perfectly clean.
Just call me retarded :)
Cargo.toml
Outdated
|
|
||
| # Unexported dependnecies | ||
| bitcoin_hashes = { version = ">=0.12, <=0.13", default-features = false } | ||
| bitcoin_hashes = { version = ">=0.12, <=0.14", default-features = false } |
There was a problem hiding this comment.
This also needs to be <0.15, it was already broken.
tcharding
force-pushed
the
07-30-update-hashes
branch
from
46ef0ed to
2b47d40
Compare
|
Oh this PR is no good anyway, |
|
Raised #79 to discuss the MSRV bump. Converting to draft until that resolves. |
|
Is this a MSRV bump? Anyone who wants an older MSRV can use an older version of As someone who currently has two copies of |
|
@kayabaNerve that line wouldn't be great, better jut use |
|
Precise updates get overriden upon the next call to update. A line in the toml, next to the line adding this very crate as a dependency, would pin it. That's why I said what I said. It's irrelevant to the larger point I don't believe this is a MSRV break and should be able to move forward regardless though. |
MSRV doesn't get bumped on update if you use recent cargo. But yes, this is not MSRV break in the sense that it'd justify major version. |
|
Could you take this PR out of draft and merge and release please? 😄 Seems like a simple change and I need it for the reason @tcharding outlined. |
I second this. The simplest fix is resting on the shelf for more than a year now. Maintainer, please do the v2.2.1 release. |
|
Ping @tcharding @stevenroose, please let's merge, it's a simple change and very much needed. Thanks |
|
I'm not a maintainer here sorry mate. If you have a direct link to Steven you could hassle him. |
|
@tcharding I guess the first step would be for you to rebase it (there are conflicts) and take it out of draft. |
tcharding
force-pushed
the
07-30-update-hashes
branch
from
2b47d40 to
1ca2e1b
Compare
Hardware devices like the smallest binary possible, currently if one builds with latest `rust-bitcoin` and `rust-bip39` they get two versions of `bitcoin_hashes` in the dependency graph because we don't support the latest `bitcoin_hashes`. Note, using the latest version causes the MSRV to increase.
tcharding
force-pushed
the
07-30-update-hashes
branch
from
1ca2e1b to
26683c2
Compare
|
No sweat, done. |
|
Oh this cannot be undrafted because of the MSRV bump. At the risk of being rude, and @stevenroose you can slap me in person when you see me next, I'd suggest one of the following:
I'm not the don of the rust-bitcoin org but I do feel in someway responsible that we have crates in the org that are not maintained. I don't know the solution to that problem. No one is required to do anything round here, so props to Steven for writing the crate, and all due respect. |
Hardware devices like the smallest binary possible, currently if one builds with latest
rust-bitcoinandrust-bip39they get two versions ofbitcoin_hashesin the dependency graph because we don't support the latestbitcoin_hashes.Note using the latest version causes the MSRV to increase.