roughike / inKino

A multiplatform Dart movie app with 40% of code sharing between Flutter and the Web.
https://inkino.app
Apache License 2.0
3.64k stars 704 forks source link

Constify inKino #76

Closed jonahwilliams closed 6 years ago

jonahwilliams commented 6 years ago

I've added const where it could be trivially swapped for new. I did indulge myself a little and swapped around widget construction in storyline_widget.dart to make more of the text widgets const.

Fixes https://github.com/roughike/inKino/issues/71

roughike commented 6 years ago

Thanks! I nitpicked about one tiny thing.

roughike commented 6 years ago

Also, is there an analysis option or similar which automatically suggests using const instead of new where appropriate?

jonahwilliams commented 6 years ago

Yes, I've added an example analysis_options.yaml which includes the prefer_const_constructors lint. This will catch all places where you could insert a const, turns out I missed a few too.

I've also added one more option in the language setting, this is a config necessary to make flutter work with the linter. Finally there is a commented out line too: implicit-dynamic: false will help make your code dart 2 safe, since it will try and prevent you from accidentally using dynamic and passing that to something that expects a more specific type. This was generally okay in dart 1, but it's not really safe. A quick example:

void takesAMap(Map<String, String> foo) { ... }
...
Map foo = {};
takesAMap(foo); // Okay in dart 1, will fail at runtime in dart 2
roughike commented 6 years ago

Thanks!

The last commit actually made the Travis build fail, and it fails locally for me as well. Do you have time to either revert it or investigate how to test errors in Futures? If not, I'm happy to continue.

jonahwilliams commented 6 years ago

That is very odd, I needed that change for the test to pass locally. Possibly due to slightly different flutter versions?

roughike commented 6 years ago

Thanks a lot! Hopefully, I'll see you in I/O to thank in person!