-
Notifications
You must be signed in to change notification settings - Fork 143
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
Update MSRV to 1.56.1 #639
Conversation
Following `rust-bitcoin` MSRV of 1.56.1, this pins MSRV to 1.56.1. Quoting @tcharding in rust-bitcoin/rust-bitcoin@d9cc724 > Rust version 1.56.0 introduced edition 2021. Shortly afterwards, on > October 21 2021 Rust version 1.56.1 was released. > > Debian stable is currently shipping `rustc 1.63.0`. > > Our stated MSRV policy is: In Debian stable and at least 2 years old. > > Therefore our MSRV policy is met by Rust version 1.56.1 and we can strat > to bump our MSRV org wide. > > Links: > - https://blog.rust-lang.org/2021/11/01/Rust-1.56.1.html > - https://blog.rust-lang.org/2021/10/21/Rust-1.56.0.html > - https://packages.debian.org/stable/rust/rustc Removing the pins in contrib/test.sh since they are not necessary in 1.56.1.
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.
ACK c301ce3
I ACKed because my local tests pass, but the real CI fails (an unsurprising outcome for a PR that's trying to change CI and not much else). |
Hey @storopoli I've sent you on a wild goose chase, I forgot we do not run the linter in CI in this repo. I think you should throw the You could put this on the attribute: |
Actually -- if the CI failures are because of missing parts from #638, then I can merge this. But I probably won't have time to check this until tomorrow. |
kludge and merge! |
What! I don't have overlord powers to change peoples PR descriptions - outrageous. @storopoli can you please remove the mention in the PR description, it makes github spam my notifications. Thanks man. |
Done, sorry about the spam. |
Just so you know, the merge script warns about this (and if you can't edit descriptions, you can't merge either, I guess you're not a maintainer here) and I always edit out |
Lol, CI won't run the MSRV job unless the nightly job also passes. That's a dumb ordering because nightly breaks on its own and MSRV only breaks if the code is wrong. Anyway I'll just run contrib.sh locally and see how things work. |
Ok, looks good. On the off chance that the pins are actually needed we can reinstate them when we're done other the other CI fixes. |
e2636f3 Clippy lints MSRV 1.56.1 (Jose Storopoli) Pull request description: Nothing out of the ordinary. The most alarming was the non-inclusive range in `test_utils.rs`. In some test functions I allowed myself to use `#[allow(clippy::type_complexity)]` because clippy was asking to create a type instead of using the big tuple. Since these were used just once, I thought it was overkill. Note, depends on: - #639 (we can also merge this into it instead of `master`) - #638 (has some allows on dead_code) ACKs for top commit: apoelstra: ACK e2636f3 Tree-SHA512: d1f38d1e2a1415da816898806377c90986072fe0bc9a9dbc3e5476026c05c53743453cbd0a237a70aa27762f1be86e44c19422575b7e62f686140accc412177c
Following
rust-bitcoin
MSRV of 1.56.1, this pins MSRV to 1.56.1.Quoting a recent merge in
rust-bitcoin
Removing the pins in
contrib/test.sh
since they are not necessaryin 1.56.1.
EDIT: CI fails because of the
#[allow(dead_code)]
implemented in #638 on some Psbt traits.Do I port these to here? Conceptually they belong to #638 and not here.