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

feat: add lint rules #1836

Merged
merged 1 commit into from
Feb 24, 2024
Merged

feat: add lint rules #1836

merged 1 commit into from
Feb 24, 2024

Conversation

josxha
Copy link
Contributor

@josxha josxha commented Feb 21, 2024

Lint rules in concideration:

  • use_decorated_box
  • use_is_even_rather_than_modulo
  • unnecessary_lambdas
  • sort_pub_dependencies
  • prefer_void_to_null
  • unnecessary_raw_strings
  • comment_references
  • parameter_assignments
  • avoid_unused_constructor_parameters

Let me know if you want any of them not included.

edit: Having all these manual lint rules it might help to concider using more restrictive lint list than the default flutter list.

@josxha josxha added this to the v7.0 milestone Feb 21, 2024
@josxha josxha self-assigned this Feb 21, 2024
@josxha josxha marked this pull request as ready for review February 21, 2024 22:48
Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

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

LGTM, I agree with all these lints. We can always add more later.

FYI the entire list of supported lints is at https://dart.dev/tools/linter-rules/all, and we can just remove the conflicting ones from there (eg. prefer single or double quotes).

@josxha
Copy link
Contributor Author

josxha commented Feb 24, 2024

FYI the entire list of supported lints is at https://dart.dev/tools/linter-rules/all, and we can just remove the conflicting ones from there (eg. prefer single or double quotes).

I'm in favour of adding more lint rules. But we might concider using another lint package and adjusting it from there to make the most out of lints as we don't really want to maintain our own lint list. Adding new lints, deactivating lints that cause issues (some lints are only experimentell and have false positivies).

There is an awesome comparison between the most common strict lint packages in this blog article. Especially the linked spreadsheet gives a really detailed comparison.

  • RydMike's list is the most restrictive. But he hasn't released it as package.
  • Lint and Very Good Analysis are probably both solid choises for restrictive rulesets. And if we don't want any rule, we can still manually override it.

@josxha josxha merged commit dda2d89 into fleaflet:master Feb 24, 2024
7 checks passed
@josxha josxha deleted the more-lints branch February 24, 2024 10:22
@JaffaKetchup
Copy link
Member

Yeah, I've seen those before. I do actually maintain my own lint set, and update it with every major Flutter release, because I don't fully trust packages to stay up to date.

@josxha
Copy link
Contributor Author

josxha commented Feb 25, 2024

I do actually maintain my own lint set, and update it with every major Flutter release

Oh I didn't knew about that. Do you have the list somewhere? I can only find it included in your packages like here.

because I don't fully trust packages to stay up to date.

That's a understandable concern, but I wouldn't be too worried since they seem to be maintained well and if they become abandoned we can simply swap them out since it is only a dev dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants