-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
bevy_core_pipeline: Apply #![deny(clippy::allow_attributes, clippy::allow_attributes_without_reason)]
#17137
base: main
Are you sure you want to change the base?
Conversation
…hout_reason)]` to bevy_core_pipeline
#![deny(clippy::allow_attributes, clippy::allow_attributes_without_reason)]
to bevy_core_pipeline#![deny(clippy::allow_attributes, clippy::allow_attributes_without_reason)]
I'll review after #16338 merged and this is updated to fix the conflict :) |
#[expect( | ||
clippy::needless_range_loop, | ||
reason = "This for-loop also uses `i` to calculate a value `t`." | ||
)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to mention: I actually spent some time seeing if I could rewrite this loop to use an iterator (as the lint suggests to do).
However, I ultimately opted not to do so, as I found that I couldn't easily get i
to be the same values without some very awkward-looking code.
I don't believe it's impossible to make an iterator work here though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe
let iter = lut_begin.ceil() as usize..=lut_end.floor() as usize
for i in iter {
}
seems a bit unnessecary tbh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what it's worth, that would silence the lint - but I suspect that that's a false negative with clippy. Hence, I don't think it would be a good idea compared to just silencing the lint as I have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those Impls seem double to me.
#[expect( | ||
clippy::needless_range_loop, | ||
reason = "This for-loop also uses `i` to calculate a value `t`." | ||
)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe
let iter = lut_begin.ceil() as usize..=lut_end.floor() as usize
for i in iter {
}
seems a bit unnessecary tbh.
This PR is blocked on #16338 per Benjamin's request. Once that is merged, I will fix any conflicts and mark it as ready to merge. |
Your PR increases Bevy Minimum Supported Rust Version. Please update the |
Objective
clippy::allow_attributes
andclippy::allow_attributes_without_reason
lints #17111Solution
Set the
clippy::allow_attributes
andclippy::allow_attributes_without_reason
lints todeny
, and bringbevy_core_pipeline
in line with the new restrictions.No code changes have been made - except if a lint that was previously
allow(...)
'd could be removed via small code changes. For example,unused_variables
can be handled by adding a_
to the beginning of a field's name.Testing
cargo clippy
andcargo test --package bevy_core_pipeline
were run, and no errors were encountered.