-
Notifications
You must be signed in to change notification settings - Fork 187
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
Move lints configuration to manifest #822
base: master
Are you sure you want to change the base?
Conversation
ash/Cargo.toml
Outdated
|
||
[lints.clippy] | ||
use_self = "warn" | ||
too_many_arguments = "allow" |
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.
Unfortunately it is not possible to override lints from the root.
So here the common lints are duplicated to be able to add the three "allow"
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 am intentionally not pushing this to the ash
repo because:
- Lints are explicitly inherited from the workspace, not implicitly forced down;
- Unnecessary MSRV bump;
- Strange duplication in overrides.
Can I close this as "Not planned"?
No problems... |
I also found it strange that workspace lints can't be overridden at the workspace member level. |
I dropped the MSRV bump. What about the lint fixes and the "resolve" warning fix ? Should we keep them ? |
I moved the PS: I don't expect this to be merged. Just showcasing. |
e33abe0
to
0bb826e
Compare
Note that everyone gets notifications when you're debugging by means of (force-)pushing. |
Sorry about that. I'll be more careful next time. |
Current status / limitations:
|
This is possible since Rust 1.74.0.
This PR shows what it looks like.