Skip to content

Conversation

@tcharding
Copy link
Member

@tcharding tcharding commented Dec 5, 2025

Use Nicks mad new tool to run CI. To do this I copied what is in rust-psbt, guessed at what bench should be, and used the latest commit hash from rust-bitcoin-maintainer-tools repo.

Note: Includes a bump of the MSRV to 1.74.0 - do we want this separate?

@tcharding
Copy link
Member Author

tcharding commented Dec 5, 2025

Probably should have an ack from @nyonson before merge, I hacked this pretty quickly without thinking super hard.

Just allow the lint to pass ignoring the large error return size.
Add import to fix the benchmarks.

Just allow the lint to pass ignoring the large error return size.
In line with `rust-bitcoin` bump the MSRV. Remove `clippy.toml` now it
is not needed with this toolchain.
@tcharding tcharding changed the title CI: Use new cargo rmbt tool CI: Use new cargo rmbt tool (incl. bump MSRV to 1.74.0) Dec 5, 2025
clippy emits:

  warning: unnecessary parentheses around closure body

As suggested remove the parentheses.
Use `to_keypair` as suggested by the linter.
Use Nicks mad new tool to run CI. To do this I copied what is in
`rust-psbt`, guessed at what `bench` should be, and used the latest
commit hash from `rust-bitcoin-maintainer-tools` repo.
@tcharding
Copy link
Member Author

tcharding commented Dec 5, 2025

@nyonson, your CI work is shit-hot mate. Well done. Want to do it in rust-bitcoin or want me to? I have already used cargo rbmt locally to test a rust-bitocin PR!

@apoelstra
Copy link
Member

In 1231487:

The commit description has some cruft about error return sizes which isn't applicable to this commit.

@apoelstra
Copy link
Member

apoelstra commented Dec 5, 2025

In f58468d:

This bumps the MSRV to 1.74, which I'd rather not do as long as we depend on rust-bitcoin 0.32. But also, you say 1.64 somewhere (typo).

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On ed9195b successfully ran local tests

@apoelstra
Copy link
Member

The MSRV will also be an issue for rust-elements.

We can attempt to back the MSRV for rbmt up, or we can figure out how we can cargo install with a newer compiler than we use to compile the actual project code.

@nyonson
Copy link
Contributor

nyonson commented Dec 5, 2025

@apoelstra as it is right now in ed9195b, it actually installs rbmt with the stable toolchain and then installs the asked for toolchain (maybe in the future we could use this "artifact dependencies" thing cargo is working on so this dance is never required). So I don't think the MSRV bump is required here for rbmt. FWIW rmbt does work with its stated 1.74 with your version fixes so I was able to simplify this for workspaces with MSRV of 1.74: https://github.com/tcharding/rust-psbt/pull/44/files

@apoelstra
Copy link
Member

Awesome. So let's try doing this one without the MSRV bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants