-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Feat/forge lint verbosity #12693
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
base: master
Are you sure you want to change the base?
Feat/forge lint verbosity #12693
Conversation
0xrusowsky
left a comment
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.
besides the lint-related comment that i left, why are you modifying cast?
please remove all of that
| .with_severity(if severity.is_empty() { None } else { Some(severity) }) | ||
| .with_severity(if severity.is_empty() { | ||
| Some(vec![Severity::Low]) | ||
| } else { | ||
| Some(severity) | ||
| }) |
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.
this is incorrect, what we should do is that when no severity is informed we exclude the Severity::Gas lints
additionally forge lint is also invoked via forge build, so rather than changing the mulitple entry points, it may be better to change the behavior in the lint crate
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.
Ah Got it. On it
Resolves #12635
Motivation
Solution
I made the severity by default to be low
PR Checklist