11.x: fix CI #835
Open
11.x: fix CI #835
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
apoelstra
force-pushed
the
2025-07_ci-0.11
branch
4 times, most recently
from
e340d80 to
a3b212e
Compare
|
This is working but I will need to rebase it after #832 because the lockfile has the old version number in it. |
apoelstra
force-pushed
the
2025-07_ci-0.11
branch
from
a3b212e to
f05624d
Compare
apoelstra
added a commit
to apoelstra/rust-miniscript
that referenced
this pull request
Jul 4, 2025
As in rust-bitcoin#835, I changed a test function that used 'A'..'Z' to use an inclusive range, which is behavior-changing, and changed the impl of OrdF64::partial_cmp to be correct. Everything else is just syntactic. I apologize for structuring this PR very differently from rust-bitcoin#835 so you can't really range-diff. I tried rebasing and there were so many conflicts that I simply redid the whole thing, and since I had some experience I made some different decisions about what order to take things in.
Aside from being more future-proof, this also works with my local CI.
It may be that ubuntu-latest breaks us, but we're not doing anything particularly exotic so it seems unlikely. Meanwhile, our use of ubuntu-20.04 HAS broken us, because that runner is no longer supported. Meanwhile also update a bunch of our action versions to v4, since some of the v2s have become deprecated and removed. (Hopefully this won't happen very often.)
apoelstra
force-pushed
the
2025-07_ci-0.11
branch
from
f05624d to
d2cdeb4
Compare
|
Rebased. |
Weird that this only has an effect on a couple files, which aren't checked in the Github CI. Same story with 12.x.
This, and the next commit, are not organized in any way except that I did a new commit partway to try to make review a bit easier. There were too many lints for me to split them all up and do them in a nice PR. This changes `OrdF64::partial_cmp` to be consistent with `OrdF64::cmp` which is a bugfix and an observable behavior change (but not an easy one to observe). All other changes are just syntax stuff.
In src/test_utils.rs I changed 'A'..'Z' to 'A'..='Z', which was a legitimate bugfix. The rest of these are just syntax stuff.
apoelstra
force-pushed
the
2025-07_ci-0.11
branch
from
d2cdeb4 to
5e6e870
Compare
|
Regarding the above "successfully ran local tests" commints -- I currently have various checks disabled on backport branches. I hope to re-enable most of them but need to get these PRs in first. |
|
Can you re-ACK Sanket? |
apoelstra
added a commit
to apoelstra/rust-miniscript
that referenced
this pull request
Jul 5, 2025
As in rust-bitcoin#835, I changed a test function that used 'A'..'Z' to use an inclusive range, which is behavior-changing, and changed the impl of OrdF64::partial_cmp to be correct. Everything else is just syntactic. I apologize for structuring this PR very differently from rust-bitcoin#835 so you can't really range-diff. I tried rebasing and there were so many conflicts that I simply redid the whole thing, and since I had some experience I made some different decisions about what order to take things in.
apoelstra
added a commit
to apoelstra/rust-miniscript
that referenced
this pull request
Jul 5, 2025
As in rust-bitcoin#835, I changed a test function that used 'A'..'Z' to use an inclusive range, which is behavior-changing, and changed the impl of OrdF64::partial_cmp to be correct. Everything else is just syntactic. I apologize for structuring this PR very differently from rust-bitcoin#835 so you can't really range-diff. I tried rebasing and there were so many conflicts that I simply redid the whole thing, and since I had some experience I made some different decisions about what order to take things in.
apoelstra
added a commit
to apoelstra/rust-miniscript
that referenced
this pull request
Jul 5, 2025
As in rust-bitcoin#835, I changed a test function that used 'A'..'Z' to use an inclusive range, which is behavior-changing, and changed the impl of OrdF64::partial_cmp to be correct. Everything else is just syntactic. I apologize for structuring this PR very differently from rust-bitcoin#835 so you can't really range-diff. I tried rebasing and there were so many conflicts that I simply redid the whole thing, and since I had some experience I made some different decisions about what order to take things in.
sanket1729
approved these changes
Jul 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Should get us green CI, and pins the nightly compiler version so that it stays green.