Skip to content

Conversation

MaikuB
Copy link
Contributor

@MaikuB MaikuB commented Jul 16, 2020

I thought I'd submit a PR to clean up some of the code to address a few issues (note: some may be consider rather opinionated so feel free to edit the changes in the PR as you see fit or close it even)

  • Use of dynamic. Especially for the Profile and Login widget where loginAction and logoutAction expects function types but no type is declared so they're dynamic
  • Inconsistencies e.g. sometimes types are declared, other times they're omitted, mix of passing double and int literals to widget size props (i.e. height/width)
  • Unawaited calls to async methods
  • Avoiding async void methods. Part of this a carry-over from best practices in C# to let the caller decide if they want await

I added an analysis_options.yaml to match the styling of the existing code (with fixes) as much as possible and make changes to address a few other issues picked up by the linter as well. One of the rules that I've commented out and haven't addressed is that the preference is to have constants in lower camel case.

Also removed the widget test as that is from the standard counter app so it's actually failing

@MaikuB
Copy link
Contributor Author

MaikuB commented Jul 16, 2020

Forgot to add, the article doesn't mention enabling refresh token rotation. Without this, if the user signed in and restarted the app and exchanged their refresh token, there'll be an exception in the initAction() method since no refresh token would be returned

@abbaspour
Copy link
Contributor

tnx @MaikuB. checking the PR. updating soon

@MaikuB
Copy link
Contributor Author

MaikuB commented Jul 17, 2020

@abbaspour no worries. Feel free to DM me if needed. There's a GDG Aus/NZ Slack group (http://bit.ly/join_gdgslack) that I'm on where you can also find me there too :)

@MaikuB
Copy link
Contributor Author

MaikuB commented Jul 22, 2020

Regarding the async void point, I forgot to mention in C#, this affected how exceptions are propagated and looks like the same applies for Dart. There's also a linter rule for Dart that says to avoid async void methods. In case you haven't come across it, the full list of rules were pulled from here. May want to consider to applying a consistent set of rules if there'll be a series of blog posts for consistency

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