Skip to content

[Merged by Bors] - fix clippy warning failing on CI #2353

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

Closed

Conversation

NathanSWard
Copy link
Contributor

@NathanSWard NathanSWard commented Jun 17, 2021

Objective

  • CI jobs are starting to fail due to clippy::bool-assert-comparison and clippy::single_component_path_imports being triggered.

Solution

  • Fix all uses where asset_eq!(<condition>, <bool>) could be replace by assert!
  • Move the #[allow()] for single_component_path_imports to #![allow()] at the start of the files.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jun 17, 2021
@NathanSWard NathanSWard added A-Build-System Related to build systems or continuous integration C-Code-Quality A section of code that is hard to understand or change and removed S-Needs-Triage This issue needs to be labelled labels Jun 17, 2021
@NathanSWard
Copy link
Contributor Author

This popped up on my recent push to #2332
See CI job here:
https://github.com/bevyengine/bevy/pull/2332/checks?check_run_id=2850651062

@NathanSWard NathanSWard added the P-High This is particularly urgent, and deserves immediate attention label Jun 17, 2021
@NathanSWard
Copy link
Contributor Author

Now I'm especially consufed.
CI is failing due to clippy::single-component-path-imports however, we explicitly have a
#[allow(clippy::single_component_path_imports)].

@NathanSWard
Copy link
Contributor Author

@mockersf any ideas?

@MinerSebas
Copy link
Contributor

Running clippy with:

cargo clippy --workspace --all-targets --all-features -- -D warnings -A clippy::type_complexity -A clippy::single_component_path_imports

Fixes that Problem.

To me this seems like a Bug in Clippy.

@NathanSWard
Copy link
Contributor Author

To me this seems like a Bug in Clippy.

Darn :/

@mockersf
Copy link
Member

It's rust-lang/rust-clippy#7290

The workaround proposed there works: add #![allow(clippy::single_component_path_imports)] at the start of src/lib.rs and crates/bevy_dylib/src/lib.rs and it will work

@NathanSWard NathanSWard changed the title fix clippy::bool-assert-comparison warning failing on CI fix clippy warning failing on CI Jun 17, 2021
@NathanSWard NathanSWard added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed C-Code-Quality A section of code that is hard to understand or change labels Jun 17, 2021
@cart
Copy link
Member

cart commented Jun 18, 2021

Looks good to me. Nice work!

@cart
Copy link
Member

cart commented Jun 18, 2021

bors r+

bors bot pushed a commit that referenced this pull request Jun 18, 2021
# Objective

- CI jobs are starting to fail due to `clippy::bool-assert-comparison` and `clippy::single_component_path_imports` being triggered.

## Solution

- Fix all uses where `asset_eq!(<condition>, <bool>)` could be replace by `assert!`
- Move the `#[allow()]` for `single_component_path_imports` to `#![allow()]` at the start of the files.
@bors bors bot changed the title fix clippy warning failing on CI [Merged by Bors] - fix clippy warning failing on CI Jun 18, 2021
@bors bors bot closed this Jun 18, 2021
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
# Objective

- CI jobs are starting to fail due to `clippy::bool-assert-comparison` and `clippy::single_component_path_imports` being triggered.

## Solution

- Fix all uses where `asset_eq!(<condition>, <bool>)` could be replace by `assert!`
- Move the `#[allow()]` for `single_component_path_imports` to `#![allow()]` at the start of the files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System Related to build systems or continuous integration P-High This is particularly urgent, and deserves immediate attention S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants