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

Add flutter lints #1306

Closed
wants to merge 3 commits into from
Closed

Conversation

julien4215
Copy link
Contributor

@julien4215 julien4215 commented Dec 24, 2024

The rules that required fixes :

  • library_private_types_in_public_api
  • unintended_html_in_doc_comment
  • use_key_in_widget_constructors
  • prefer_const_literals_to_create_immutables
  • prefer_const_constructors

I am not sure if we really want to add this set of rules because some of them seem to especially address to packages like unintended_html_in_doc_comment, use_key_in_widget_constructors, library_private_types_in_public_api. However I think it is still interesting to keep the rules on const.

So I see two options, We can either add manually the two rules on const constructor or add the flutter set of rules and remove the ones that seem specific to Flutter packages.

- package:flutter_lints/flutter.yaml
- package:lint/strict.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to keep both lint packages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends on what we want.

What do you think about these three rules : library_private_types_in_public_api, unintended_html_in_doc_comment, use_key_in_widget_constructors ? Should we add them ? If not I guess that we can keep only the lint package.

Copy link
Contributor

Choose a reason for hiding this comment

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

use_key_in_widget_constructors we definitely should have it, and library_private_types_in_public_api looks like a good practice too.

the third one we don't really care.

We should pick the set of rules that best fit our needs, and then adapt by manually enabling/disabling rule individually. Flutter lints is certainly a good choice so we can keep it. Have you found all the differences between flutter_lints and lint/strict?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes there is google sheets at the end of this article that show the differences but this is probably not up to date.

I'll disable the rule about html doc comment.

@veloce
Copy link
Contributor

veloce commented Dec 25, 2024

Ok so lint the package we're currently using uses 78% of the rules, and flutter_lint uses only 48%.

So we should keep lint and add the missing flutter rules we need manually. Another interesting set of rules could be the flutter master repository which also has 78% according to that article.

@julien4215
Copy link
Contributor Author

What's the point of adding it flutter_lint rules manually ?

I guess the best solution would be to set all lint rules manually like they do in the flutter master repository so we have exactly what we want but that would take some time.

@julien4215 julien4215 marked this pull request as draft December 26, 2024 09:24
@veloce
Copy link
Contributor

veloce commented Dec 26, 2024

What's the point of this PR?

I thought that you realised some useful rules were missing. If that is the case, change the PR to manually enable the missing rules while keeping the inclusion of lint/strict set of rules.

@julien4215
Copy link
Contributor Author

Well I thought that it would add useful rules because lint/strict is only about the Dart language and not Flutter but I realize that was a mistake.

The rules prefer_const_literals_to_create_immutables and prefer_const_constructors are already in the set of rules lint/strict and the rules library_private_types_in_public_api, use_key_in_widget_constructors would only be useful if this was a package. So I'll close this PR.

@julien4215 julien4215 closed this Dec 26, 2024
@julien4215 julien4215 deleted the flutter-lints branch December 26, 2024 10:36
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