Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clippy: fix extra errors and enforce target-applies-to-host in xtask #3497

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

bnjbvr
Copy link
Member

@bnjbvr bnjbvr commented Jun 3, 2024

This enforces the env variable to set the --target-applies-to-host setting when running in xtask. I'm not sure why it's required, but without it, the CI step didn't find all the issues that I could find locally. @jplatte if you have any idea about that, I'd be curious; I've mostly cargo culted the existing workaround used when building the doc. It looks like another instance of rust-lang/cargo#10744 in a slightly different context.

The second commit fixes the issues found by clippy.

Without this, the configs from `.cargo/config.toml` were not read in CI
tasks, causing false positives when running Clippy on CI (i.e. there
were issues observed when compiling locally that were not found when
compiling remotely).

Not entirely sure why it's needed, because I'm seeing the issues when
I'm using `cargo xtask ci clippy` locally, with nothing changed.
@bnjbvr bnjbvr force-pushed the bnjbvr/fix-local-clippy-errors branch from 0175cdf to 276b6d7 Compare June 3, 2024 15:24
We should investigate why some errors were not found in CI tasks,
though.
@bnjbvr bnjbvr force-pushed the bnjbvr/fix-local-clippy-errors branch from 276b6d7 to 313af41 Compare June 3, 2024 15:24
@bnjbvr bnjbvr changed the title clippy: fix errors found locally clippy: fix extra errors and enforce target-applies-to-host in xtask Jun 3, 2024
@bnjbvr bnjbvr marked this pull request as ready for review June 3, 2024 15:24
@bnjbvr bnjbvr requested a review from a team as a code owner June 3, 2024 15:24
@bnjbvr bnjbvr requested review from Hywan, jplatte and a team and removed request for a team June 3, 2024 15:24
Copy link

codecov bot commented Jun 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.73%. Comparing base (78608a1) to head (313af41).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3497   +/-   ##
=======================================
  Coverage   83.73%   83.73%           
=======================================
  Files         255      255           
  Lines       25778    25778           
=======================================
  Hits        21584    21584           
  Misses       4194     4194           

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

Copy link
Member

@Hywan Hywan left a comment

Choose a reason for hiding this comment

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

It looks sensible to me, thanks for your investigation.

@bnjbvr bnjbvr merged commit 9d46da9 into main Jun 5, 2024
46 of 58 checks passed
@bnjbvr bnjbvr deleted the bnjbvr/fix-local-clippy-errors branch June 5, 2024 07:34
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.

2 participants