Skip to content

bevy_render: Apply #![deny(clippy::allow_attributes, clippy::allow_attributes_without_reason)] #17194

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

Conversation

LikeLakers2
Copy link
Contributor

Objective

Solution

Set the clippy::allow_attributes and clippy::allow_attributes_without_reason lints to deny, and bring bevy_render in line with the new restrictions.

Testing

cargo clippy and cargo test --package bevy_render were run, and no errors were encountered.

#![expect(
clippy::module_inception,
reason = "The parent module contains all things viewport-related, while this module handles cameras as a component."
)]
Copy link
Contributor Author

@LikeLakers2 LikeLakers2 Jan 6, 2025

Choose a reason for hiding this comment

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

The issue of the module sharing a name with its parent is out-of-scope for this PR; but I have created an issue on the matter: #17196

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue number should go in the reason comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, let me do that.

Comment on lines +241 to +244
#[expect(
clippy::match_same_arms,
reason = "LoadedWithDependencies is marked as a TODO, so it's likely this will no longer lint soon."
)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not a fan of this reason... but I also didn't know what else to do, because merging the two arms would mean removing the // TODO comment.

Comment on lines +919 to +922
#[expect(
clippy::match_same_arms,
reason = "LoadedWithDependencies is marked as a TODO, so it's likely this will no longer lint soon."
)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ditto.

@BenjaminBrienen BenjaminBrienen added D-Trivial Nice and easy! A great choice to get started with Bevy A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 6, 2025
@BenjaminBrienen BenjaminBrienen 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 S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 6, 2025
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

This is an improvement, although I agree with wanting to clean up a couple of those.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 6, 2025
Merged via the queue into bevyengine:main with commit 27802e6 Jan 6, 2025
29 checks passed
@LikeLakers2 LikeLakers2 deleted the lint/deny_allow_and_without_reason/bevy_render branch January 7, 2025 02:38
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 4, 2025
…ttributes_without_reason)]` (bevyengine#17194)

# Objective
- bevyengine#17111

## Solution
Set the `clippy::allow_attributes` and
`clippy::allow_attributes_without_reason` lints to `deny`, and bring
`bevy_render` in line with the new restrictions.

## Testing
`cargo clippy` and `cargo test --package bevy_render` were run, and no
errors were encountered.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants