Skip to content
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

chore: Fix some beta clippy lints #16679

Merged
merged 2 commits into from
Jun 12, 2024

Conversation

torokati44
Copy link
Member

Not all of them are fixed, see: rust-lang/rust-clippy#12917

@torokati44 torokati44 changed the title Fix some beta clippy lints chore: Fix some beta clippy lints Jun 11, 2024
@torokati44
Copy link
Member Author

Thanks for the approval, @kjarosh! I've added yet another fix, and a suppression, to make it pass cleanly.

@kjarosh
Copy link
Member

kjarosh commented Jun 11, 2024

Regarding the suppression I'm wondering whether that's the best way we can address it. The problem is caused by some commented out constants, so if we need a suppression anyway, maybe it's better to uncomment these constants and allow dead code only for them?

If I understand it correctly, the current suppression does not address the fact that the docs for commented out constants will be part of UNIT_CIRCLE_POINTS's docs?

@torokati44
Copy link
Member Author

maybe it's better to uncomment these constants and allow dead code only for them?

Yes, I tried that, but unfortunately sin is not allowed in floating point constants (anymore?)... :/

@torokati44
Copy link
Member Author

If I understand it correctly, the current suppression does not address the fact that the docs for commented out constants will be part of UNIT_CIRCLE_POINTS's docs?

Well, it depends on what tools you ask... 😵‍💫

@torokati44
Copy link
Member Author

Anyway, removed the suppression for now, will submit it as a separate PR.

@Dinnerbone Dinnerbone merged commit 0d25b5d into ruffle-rs:master Jun 12, 2024
15 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants