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

Improve codestyle of project #754

Closed
3 tasks
tobrun opened this issue Nov 13, 2021 · 5 comments
Closed
3 tasks

Improve codestyle of project #754

tobrun opened this issue Nov 13, 2021 · 5 comments

Comments

@tobrun
Copy link
Collaborator

tobrun commented Nov 13, 2021

This community driven project doesn't have a unified code style. We, or at least I myself, have been very forgiven when reviewing pull requests. I didn't want to waste anyone time with having to make additional iterations on their code before being able to merge. This approach however doesn't scale and makes the project long term harder to maintain and read. To improve this, we should define some basic style guides for all the languages that this project supports.

Definition of done:

For each programming language we enforce code style with static code analysis in a CI env.

Work to be done:

For each supporting programming language

  • define a codestyle
  • add static code analysis integration
  • run static code analysis in CI env.
@felix-ht
Copy link
Collaborator

felix-ht commented Dec 3, 2021

Lint rules i would like to see added to the basic flutter_lints:

  • always_declare_return_types
  • prefer_single_quotes
  • sort_child_properties_last
  • unawaited_futures

@m0nac0
Copy link
Collaborator

m0nac0 commented Dec 3, 2021

The new check added in #803 only checks Dart formatting, we already have a lint analysis step. But that only analyzes the example app, not the entire plugin, since there used to be some issues with that. We should try to make it analyze the whole project again.

Lint rules i would like to see added to the basic flutter_lints:

* always_declare_return_types

* prefer_single_quotes

* sort_child_properties_last

* unawaited_futures

Those all seem reasonable, though I personally prefer double quotes. I guess there isn't a clear preference in the Dart community, since there is also a prefer_single_quotes rule?

What I'm a bit worried about regarding adding special rules: it makes contributing harder. Most devs should be familiar with the standard flutter lints.

But both of these aren't strong opionions of mine.

@felix-ht
Copy link
Collaborator

felix-ht commented Dec 3, 2021

always_declare_return_types - is something that usually gets fixed in code reviews - better to have a linter point it out.
prefer_single_quotes - this one is not overly important - might be nice tho to agree on either single or double quotes. Most of the code currently uses double quotes so we can also go with that
sort_child_properties_last - not super important but makes for cleaner code
unawaited_futures - is something that helped me out personally - without it's super easy to miss awaiting a future

On the point of making contributing harder - with the solid integration of the linter into the IDEs, the IDE will point out the issues without any other action from the contributor.

@m0nac0
Copy link
Collaborator

m0nac0 commented Dec 3, 2021

the IDE will point out the issues without any other action from the contributo

Sure, my point was that some new contributors might not know how to fix some of these more rarely used lints (shouldn't be an issue for these 4, but might be if we add others).

But even then, someone else could just help out with the PR. So I agree it's actually not an issue.

@tobrun
Copy link
Collaborator Author

tobrun commented Dec 16, 2021

Final tailwork in #818, closing overarching issue as we have dart/swift:
image

@tobrun tobrun closed this as completed Dec 16, 2021
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

No branches or pull requests

3 participants