Re-write CI #338
Closed
Re-write CI #338
Conversation
This file contains 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
tcharding
force-pushed
the
04-30-rewrite-ci
branch
5 times, most recently
from
3487a2a
to
7b21f8d
Compare
|
|
tcharding
force-pushed
the
04-30-rewrite-ci
branch
3 times, most recently
from
9fba438
to
bffc9a6
Compare
|
Woops I got mixed up, this crate runs the formatter not the linter in CI ... |
tcharding
force-pushed
the
04-30-rewrite-ci
branch
2 times, most recently
from
91e0049
to
94dbda8
Compare
tcharding
force-pushed
the
04-30-rewrite-ci
branch
from
94dbda8
to
4b2a5c5
Compare
This was referenced Apr 30, 2024
tcharding
force-pushed
the
04-30-rewrite-ci
branch
from
4b2a5c5
to
aefaa9c
Compare
Merged
tcharding
force-pushed
the
04-30-rewrite-ci
branch
from
aefaa9c
to
2c10be4
Compare
tcharding
added a commit
to tcharding/rust-bitcoincore-rpc
that referenced
this pull request
May 1, 2024
In rust-bitcoin#326 we changed a function to use `bitcoin::sign_message::MessageSignature` but doing so requires the "base64" feature to be on for `bitcoin`. This did not get caught by CI but improvements to CI in rust-bitcoin#338 will now catch this. Add a feature to `json` and `client` that allows enabling "base64" if the `verifymessage` RPC call is required.
tcharding
added a commit
to tcharding/rust-bitcoincore-rpc
that referenced
this pull request
May 1, 2024
In rust-bitcoin#326 we changed a function to use `bitcoin::sign_message::MessageSignature` but doing so requires the "base64" feature to be on for `bitcoin`. This did not get caught by CI but improvements to CI in rust-bitcoin#338 will now catch this. Add a feature to `json` and `client` that allows enabling "base64" if the `verifymessage` RPC call is required.
tcharding
force-pushed
the
04-30-rewrite-ci
branch
from
2c10be4
to
6c133a3
Compare
MSRV build just broke because of a bunch of dependencies. I did not investigate why I just found a set of versions that builds.
In rust-bitcoin#326 we changed a function to use `bitcoin::sign_message::MessageSignature` but doing so requires the "base64" feature to be on for `bitcoin`. This did not get caught by CI but improvements to CI in rust-bitcoin#338 will now catch this. Add a feature to `json` and `client` that allows enabling "base64" if the `verifymessage` RPC call is required.
tcharding
added a commit
to tcharding/rust-bitcoincore-rpc
that referenced
this pull request
May 1, 2024
In rust-bitcoin#326 we changed a function to use `bitcoin::sign_message::MessageSignature` but doing so requires the "base64" feature to be on for `bitcoin`. This did not get caught by CI but improvements to CI in rust-bitcoin#338 will now catch this. Add a feature to `json` and `client` that allows enabling "base64" if the `verifymessage` RPC call is required.
tcharding
added a commit
to tcharding/rust-bitcoincore-rpc
that referenced
this pull request
May 1, 2024
In rust-bitcoin#326 we changed a function to use `bitcoin::sign_message::MessageSignature` but doing so requires the "base64" feature to be on for `bitcoin`. This did not get caught by CI but improvements to CI in rust-bitcoin#338 will now catch this. Add a feature to `json` and `client` that allows enabling "base64" if the `verifymessage` RPC call is required.
tcharding
force-pushed
the
04-30-rewrite-ci
branch
from
6c133a3
to
f752adc
Compare
tcharding
added a commit
to tcharding/rust-bitcoincore-rpc
that referenced
this pull request
May 1, 2024
In rust-bitcoin#326 we changed a function to use `bitcoin::sign_message::MessageSignature` but doing so requires the "base64" feature to be on for `bitcoin`. This did not get caught by CI but improvements to CI in rust-bitcoin#338 will now catch this. Add a feature to `json` and `client` that allows enabling "base64" if the `verifymessage` RPC call is required.
Currently in CI the formatter is being run with `--check` but a non-zero return status is not causing the script to fail so failure is not causing any CI job to fail. Inn preparation for correctly running the formatter in CI manually run it using `cargo fmt`.
Re-write CI in a similar manner to what we did in `rust-bitcoin` recently. Some benefits: - The `contrib/run_task.sh` can be used from the command line. - GitHub action is dumber i.e., all the logic is in the shell script instead of spread between the action and the script. - The action yaml is [hopefully] a bit cleaner. Covearge is the same, three toolchains, linter, integration tests for various Core versions. Includes a bunch of pinning for MSRV that seems to currently be broken, I'm not exactly sure when it broke.
tcharding
force-pushed
the
04-30-rewrite-ci
branch
from
f752adc
to
980e0b3
Compare
|
Replaced by #348 |
tcharding
added a commit
to tcharding/rust-bitcoincore-rpc
that referenced
this pull request
May 2, 2024
In rust-bitcoin#326 we changed a function to use `bitcoin::sign_message::MessageSignature` but doing so requires the "base64" feature to be on for `bitcoin`. This did not get caught by CI but improvements to CI in rust-bitcoin#338 will now catch this. Add a feature to `json` and `client` that allows enabling "base64" if the `verifymessage` RPC call is required.
apoelstra
added a commit
that referenced
this pull request
May 9, 2024
93602a9 CI: Use script from rust-bitcoin-maintainer-tools (Tobin C. Harding) 41c457e CI: Reduce indentation (Tobin C. Harding) Pull request description: Use the new maintainer tools test script we created in rust-bitcoin/rust-bitcoin-maintainer-tools#4 For this repo usage of the script is a big change, the coverage does not change that much except we run one example instead of just building it and we run cargo using `cargo --locked` whereas currently for stable and nightly the dependencies used are much more recent. FTR: - We do not format (exists in current ci but is not enforced because of bug is `test.sh`) - We just build the examples (same as current behaviour) - We do not lint (same as current behaviour) - We do not pin, instead we commit a lock file with working dependency versions. Please note that there are two lock files as is customary but in this repo I was unable to get a set of more recent dependencies to build so both files are the same. This replaces #338 ACKs for top commit: apoelstra: ACK 93602a9 Tree-SHA512: 67a298fddb670714fd27736341b2aa922208d45baadffefc7a9b8dc2ebd48d551b8fc36426bea06d89cddd901ae7feeaf0ebe16ec4668cc3066056f9bb6cd580
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.
Draft bebitcoin/rust-bitcoin-maintainer-tools/pull/4, after that PR merges we will need to update this to use master on
rust-bitcoin-maintainer-tools(currently using the branch on my fork).cause on top of #343Note the current CI script does not enforce
rustfmtbecause the script does not exit with non-zero ifcargo fmt --all -- --checkreturns non-zero return status - this is a bug.