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

Enable some clippy lints #110

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Enable some clippy lints #110

wants to merge 8 commits into from

Conversation

pczarn
Copy link
Contributor

@pczarn pczarn commented Jul 16, 2024

No description provided.

@pczarn
Copy link
Contributor Author

pczarn commented Jan 26, 2025

@Jujumba Could you review this? Any ideas on this?

@pczarn
Copy link
Contributor Author

pczarn commented Jan 26, 2025

We might find some more Clippy lints fit for our projects. They do run in the CI.

Thanks in advance.

Copy link
Contributor

@Jujumba Jujumba left a comment

Choose a reason for hiding this comment

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

Looks good, but I personally use variable shadowing from time to time, so not quite sure how this will affect future contributions. Perhaps we should write CONTRIBUTIONS.md guide, or whatever?

@Jujumba
Copy link
Contributor

Jujumba commented Jan 28, 2025

I propose we also apply the following lints:

Checks for matches with a single arm where an if let will usually suffice.

Suggests the use of const in functions and methods where possible.

Suggests the use of const in functions and methods where possible.

@pczarn

@pczarn
Copy link
Contributor Author

pczarn commented Jan 28, 2025

Also

multiple_bound_locations 📋
suspicious warn
What it does

Check if a generic is defined both in the bound predicate and in the where clause.

multiple_inherent_impl

@pczarn
Copy link
Contributor Author

pczarn commented Jan 29, 2025

The const fn lint should not be used on stable. We're working on stable.

@Jujumba Jujumba mentioned this pull request Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants