Closed rydmike closed 3 years ago
weta-vn
commented
3 years ago @rydmike We applied your linting rules in this commit. It's very fun and useful for optimizing our code. Thank you very much. https://github.com/weta-vn/advance_image_picker/tree/dev_/linting
rydmike
commented
3 years ago Wow @weta-vn, that was quick work! 👍🏻
Excellent and glad that you liked the idea. Yes using linting in Flutter projects and packages is very useful for keeping code quality consistent and high. I highly recommend it for all Flutter projects.
Looking forward to checking out what the new code base looks like after this update.
I might have some other suggestions and ideas for the picker later too. More related to features and minor tuning of the picker. 😃
Linting Proposal for advance_image_picker
I suggested in discussions elsewhere that it could be a good idea to add linting to this package to ensure a consistent style and minimize the risk of potential issues and bugs.
So what is Dart linting?
You can read more about it my blog here: https://rydmike.com/blog_flutter_linting Remi (Provider, Riverpod etc author) also wrote about it here: https://dash-overflow.net/articles/getting_started/
In Dart docs you can read about it here: https://dart.dev/guides/language/analysis-options#enabling-linter-rules
In next release of Flutter a new default set of linting rules will be enabled by default for new packages, read about it here: https://flutter.dev/docs/release/breaking-changes/flutter-lints-package
Setup for this package?
This package already has a lot of dependencies, so I suggest making your own lint setup that you can control and modify easily as described in my and Remi's articles. Then there is no need for any extra or new default package for it.
I made an example setup in my fork of the package here: https://github.com/rydmike/advance_image_picker/commits/linting-suggestion
Basically it just adds two new files:
I did not make any corrections that would be needed to comply with this linting setup. I just wanted to see what it would result in. As can be seen, with this configuration and rule set, the analyzer detects 110 "errors" and 1 hint. The errors are not necessarily real or bad errors, they just do not comply with the stringent linting rules configured in this example.
Next steps?
If you just want to try and see for yourself what happens when you apply these rules, well then just copy the above mentioned files to the root of your project. When you are done looking at it you can just remove the files again, the rules themselves do not make any changes. But you won't be able to build the project with this rule set enabled, since a lot is flagged as errors by it, unless you start making the required fixes until it is all fixed and it builds again.
Naturally I did not make any pull request for this, it is just an example. Adding this rule set and fixing all the lint analysis errors is a fairly big task.
If you are interested in it I could certainly attempt it and do the fixes, and submit a pull request. However, I think it would probably be better if you take a look at it first yourself. Also when making a change like this, it is better if you don't have any active dev branches that you plan to merge later, as that would be quite problematic.
If you at some point don't have any active dev branches and want help with a migration like this, I could certainly help you with it. I have done it many times before, in much bigger projects than this this too. Yes, it is a bit tedious to migrate, but really nice to have the benefits of all this once it is done. Some of the analyzer issues Dart can also fix automatically with a tool, after you applied the new lint/analyzer rules, but not everything.
Anyway, let me know how you want to proceed and if and when you want any help with it.
Again, I love this package, just want to help make it even better! 😃
BR Mike