Skip to content

ci: enable workspace lint rules#963

Open
nyonson wants to merge 1 commit into
rust-bitcoin:masterfrom
nyonson:enable-workspace-lint
Open

ci: enable workspace lint rules#963
nyonson wants to merge 1 commit into
rust-bitcoin:masterfrom
nyonson:enable-workspace-lint

Conversation

@nyonson
Copy link
Copy Markdown
Contributor

@nyonson nyonson commented May 22, 2026

First patch defines the workspace lint rules and enables them in the root package. While I believe the rules could just be defined in the root package directly, might be nice to share them with the fuzz package (and any future packages)?

Closes #959

@nyonson
Copy link
Copy Markdown
Contributor Author

nyonson commented May 22, 2026

I'll tack on a commit to fix up the lints if cool with this approach.

@trevarj
Copy link
Copy Markdown
Contributor

trevarj commented May 22, 2026

Should we bump the rbmt-version commit?

@nyonson
Copy link
Copy Markdown
Contributor Author

nyonson commented May 22, 2026

Yea, let me see how easy it is to update to v0.3.0

@apoelstra
Copy link
Copy Markdown
Member

concept ACK.

Interesting to just turn on use_self completely. In my version of that patch I'd ignored a lot of instances (e.g. for Pkh which is shorter to type than Self). But I noticed Tobin just fixed every instance and that's fine too (and maybe easier to maintain).

The lint is particularly useful here because we have a lot of generics, and Self sets them all while just Miniscript lets the type inference engine set them, which can be confusing for both the reviewer and the compiler.

Comment thread Cargo.toml
Comment on lines +78 to +79
[lints]
workspace = true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Gee wiz, so this is the only thing I was missing. Face palm.

@tcharding
Copy link
Copy Markdown
Member

How about I grab this patch then the ones from #944 sans the re-name and we push that up first? (And I'll include the 'stop using ::*` thing as suggested on that PR.)

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.

Linting isn't configured to work with cargo rbmt ATM

4 participants