-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Make ForceWarn a lint level. #86009
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
Make ForceWarn a lint level. #86009
Conversation
r? @davidtwco (rust-highfive has picked a reviewer for you, use r? to override) |
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 like that this uses the existing infrastructure, but I personally find it a tiny bit more complicated to reason about since --force-warns
isn't really a logical level. But I'm happy either way. 😊
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.
Apologies for the delay in reviewing, I feel like this is an improvement to me, but I appreciate that it isn't logically a lint level. r=me if you and @rylev are happy with it.
I really have trouble understanding why you consider it as not being a lint level. The way I see it, |
Hmmm My issue is that I believe my issue is one of nomenclature. From the command line, the user is logically forcing a certain lint to warn so the name "force-warn" makes sense. But at the lint definition site, if you sent I'm largely in favor this change now, but two questions:
|
☔ The latest upstream changes (presumably #86627) made this pull request unmergeable. Please resolve the merge conflicts. |
r? @rylev |
Yes
I chose
I will do both changes at once if you decide to change the name. |
@cjgillot Actually, let's merge this first. We can bike shed the name in a new PR as part of stabilization. |
@bors r+ |
📌 Commit e42271d has been approved by |
☀️ Test successful - checks-actions |
This seems to have broken interaction with
The lints no longer fire on an example: #![allow(ellipsis_inclusive_range_patterns)]
pub fn f() -> bool {
let x = 123;
match x {
0...100 => true,
_ => false,
}
} This is somewhat different from #86572, since Can you take a look at this? |
The interaction with cap-lints has been fixed in #86572. |
Follow-up to #85788
cc @rylev