Skip to content

Conversation

@LikeLakers2
Copy link
Contributor

Objective

Solution

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

Testing

cargo clippy --tests --all-features --package bevy_gltf was run, and no errors were encountered.

)]
#[allow(
unused_variables,
reason = "Depending on what features are used to compile this crate, certain parameters may end up unused."
Copy link
Contributor

Choose a reason for hiding this comment

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

You can add feature gates to parameters. This is a usability improvement for callers using those feature combinations.

Copy link
Contributor Author

@LikeLakers2 LikeLakers2 Jan 10, 2025

Choose a reason for hiding this comment

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

ClearcoatExtension (the struct this lint is in) is not a public struct, and this method is only called in one place; thus, there wouldn't be much of a point.

Copy link
Contributor

Choose a reason for hiding this comment

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

Im general, if the values at the call-sites aren't needed, it means that the caller can do less work to prepare arguments for the function. See how we festure gate render and text within bevy-ui. I'll approve this since it doesn't need to be addressed in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Yeah, even if we wanted to feature-gate it, I feel like that'd be work for a different PR.

)]
#[allow(
unused_variables,
reason = "Depending on what features are used to compile this crate, certain parameters may end up unused."
Copy link
Contributor

Choose a reason for hiding this comment

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

You can add feature gates to parameters. This would be a usability improvement for callers using those feature combinations.

Copy link
Contributor Author

@LikeLakers2 LikeLakers2 Jan 10, 2025

Choose a reason for hiding this comment

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

AnisotropyExtension (the struct this lint is in) is not a public struct, and this method is only called in one place; thus, there wouldn't be much of a point.

@BenjaminBrienen BenjaminBrienen added D-Trivial Nice and easy! A great choice to get started with Bevy S-Needs-Review Needs reviewer attention (from anyone!) to move forward C-Code-Quality A section of code that is hard to understand or change A-glTF Related to the glTF 3D scene/model format labels Jan 10, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 10, 2025
Merged via the queue into bevyengine:main with commit 8a82a0c Jan 10, 2025
28 checks passed
@LikeLakers2 LikeLakers2 deleted the lint/deny_allow_and_without_reason/bevy_gltf branch January 10, 2025 20:24
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 4, 2025
…ributes_without_reason)]` (bevyengine#17280)

# Objective
- bevyengine#17111

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

## Testing
`cargo clippy --tests --all-features --package bevy_gltf` was 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-glTF Related to the glTF 3D scene/model format 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-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants