Enable unused_qualifications lint at the workspace level
#14782
Labels
A-Build-System
Related to build systems or continuous integration
C-Feature
A new feature, making something new possible
D-Trivial
Nice and easy! A great choice to get started with Bevy
S-Ready-For-Implementation
This issue is ready for an implementation PR. Go for it!
X-Contentious
There are nontrivial implications that should be thought through
Comments
alice-i-cecile
added
C-Feature
This was referenced Aug 17, 2024
github-merge-queue bot
pushed a commit
that referenced
this issue
Aug 19, 2024
# Objective While looking through the changes #14782 will create I noticed this. ## Solution Reuse the existing thread_rng. As this is a code change I would like to not include it in a pure lint enable PR. ## Testing I did not test this change (other than the automated CI with this PR). I think it should be a fairly simple change that can be reviewed only by the code.
github-merge-queue bot
pushed a commit
that referenced
this issue
Aug 21, 2024
# Objective Fixes #14782 ## Solution Enable the lint and fix all upcoming hints (`--fix`). Also tried to figure out the false-positive (see review comment). Maybe split this PR up into multiple parts where only the last one enables the lint, so some can already be merged resulting in less many files touched / less potential for merge conflicts? Currently, there are some cases where it might be easier to read the code with the qualifier, so perhaps remove the import of it and adapt its cases? In the current stage it's just a plain adoption of the suggestions in order to have a base to discuss. ## Testing `cargo clippy` and `cargo run -p ci` are happy.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What problem does this solve or what need does it fill?
As seen in #10749, redundant imports can be quite messy.
What solution would you like?
As pointed out to me, the
unused_qualificationsRust lint checks for this. It's allow-by-default, so we just need to turn it on. This should be set towarn, like the rest of our lints.What alternative(s) have you considered?
We can continue to check and clean this up manually.
Additional context
The text was updated successfully, but these errors were encountered: