Deny warnings in CI

[tnull]

Since we recently got rid of our build/test/doc warnings, we now deny warnings via -D warnings in CI, enforcing no new ones will have to be fixed where they are introduced.

We also fix the last few warnings that only occured under certain feature configurations.

[tnull]

Draft since I'm still playing whack-a-mole with warnings under certain feature configs.

[TheBlueMatt]

IMO we should do this on a fixed rustc version, as new versions (esp beta) may introduce new warnings and we don't need to fail for them.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4deb263) 88.46% compared to head (71748d3) 88.47%.

❗ Current head 71748d3 differs from pull request most recent head 2d6464c. Consider uploading reports for the commit 2d6464c to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2788   +/-   ##
=======================================
  Coverage   88.46%   88.47%           
=======================================
  Files         114      114           
  Lines       91806    91814    +8     
  Branches    91806    91814    +8     
=======================================
+ Hits        81214    81229   +15     
+ Misses       8112     8100   -12     
- Partials     2480     2485    +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tnull]

IMO we should do this on a fixed rustc version, as new versions (esp beta) may introduce new warnings and we don't need to fail for them.

Not sure how often warnings that made it into the beta channel are not going to be in stable down the line. That said, now switched to only enforce it in stable CI. I also took the liberty of excluding some cases (taproot, async-signing, but also liberally added allows to the log-level tests). If CI now passes this should be ready for review.

[TheBlueMatt]

Ah I meant I think we should fail but only on a specific version of rustc pinned, rather than stable/beta.

[tnull]

Ah I meant I think we should fail but only on a specific version of rustc pinned, rather than stable/beta.

Ah, but part of the motivation here is to get these warnings fixed ASAP once they appear? Note that the only fixed version we have available in our CI would be the MSRV, and I think for this purpose (improving dev UX, cleanup the code as we go) the stable channel seems more suited to me?

[TheBlueMatt]

If we want to do that we should run this as a cron job. We have enough issues with CI on PRs randomly failing, I'd really like to avoid introducing new ones.

[tnull]

If we want to do that we should run this as a cron job. We have enough issues with CI on PRs randomly failing, I'd really like to avoid introducing new ones.

Running as a cron job really makes no sense to me as we'll want to enforce that warnings are fixed before PRs get merged. I don't think it's common that a new stable release introduces new warnings? So not entirely sure if this would be an issue at all?

In any case, since you seem to feel strongly about this I now switched to check it on 1.63.0, which, while I don't think is optimal, at least should reduce the most common stuff such as unused imports, etc.

[TheBlueMatt]

LGTM. The changes are only CI, code deletion, some use rewrites, and a few cfg tags. I don't see a strong need to waste another reviewer's time on this.