-
Notifications
You must be signed in to change notification settings - Fork 97
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
fix(lints): enable linter rules #191
Conversation
constant_identifier_names: false | ||
always_declare_return_types: true |
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.
rules are by default enabled unless explicitly disabled.
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.
Nope. You can see the changes. Earlier when these rules are not mentioned as true there were no issue otherwise CI would fail at the flutter analyze .
step.
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.
The default rules and requirements are already enabled. Explicitly enabling more rules would cause more errors and stricter rules for building.
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.
Linters are basically meant for that only to enforce best practices. And I am not explicitly enabling rules from my side, the above was mentioned in the issue.
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.
Nope. You can see the changes. Earlier when these rules are not mentioned as true there were no issue otherwise CI would fail at the
flutter analyze .
step.
So, you are saying that if any rule is not set to be true. By default they are not checked by linter ?
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.
So this confusion everyone had, you are right that we have to enable them as the specs of analyzer is designed in this way.
But actually, I meant that you don't need to write enable statements since they are already included by flutter_lints
package (check include statement in analysis_options.yaml:1
) and in turn flutter_lints
uses dart-lang/lints
's recommended linter rules.
I hope that clears the doubt ?
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.
So this confusion everyone had, you are right that we have to enable them as the specs of analyzer is designed in this way.
But actually, I meant that you don't need to write enable statements since they are already included by
flutter_lints
package (check include statement inanalysis_options.yaml:1
) and in turnflutter_lints
usesdart-lang/lints
's recommended linter rules.I hope that clears the doubt ?
Yeah, got it @manjotsidhu. So, I am closing this PR.
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.
Nope. You can see the changes. Earlier when these rules are not mentioned as true there were no issue otherwise CI would fail at the
flutter analyze .
step.So, you are saying that if any rule is not set to be true. By default they are not checked by linter ?
Yes, all the linter rules specified in the issue are not checked by default. We have to specify them.
Fixes #149
Describe the changes you have made in this PR -
In the previous pull requests of this issue, linter rules are fixed and removed from
analysis_options.yaml
. They must be marked as true to enable them.Screenshots of the changes (If any) -
Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.