refactor: packages/alice #199
Conversation
| @@ -1,16 +1,10 @@ | |||
| 1include: package:very_good_analysis/analysis_options.yaml | |||
| analyzer: | |||
There was a problem hiding this comment.
What is the reason of using flutter_lints instead of very_good_analysis?
There was a problem hiding this comment.
One is standard and always up to date, the other one is custom and outdated (8 months +). It also pointed to some potential issues like using contexts across async gaps, using forEach where a for loop should be used, const optimizations, etc.
| } | ||
| } | ||
|
|
||
| /// Get context from navigator key. Used to open inspector route. | ||
| BuildContext? getContext() => navigatorKey?.currentState?.overlay?.context; | ||
|
|
||
| String _getNotificationMessage() { | ||
| final calls = callsSubject.value; | ||
| final successCalls = calls | ||
| final List<AliceHttpCall> calls = callsSubject.value; |
There was a problem hiding this comment.
I'm just thinking about this change - what's the reason of adding type of variable here? Is it for readability or is there any other reason?
There was a problem hiding this comment.
This question is also for other changes in this PR where you've added type for the variable.
There was a problem hiding this comment.
Readability and conciseness. It's easier to understand what type of variable is this way without the help of an IDE when reviewing it on Github, for example.
| } | ||
|
|
||
| final Directory? externalDir = switch (Platform.operatingSystem) { | ||
| "android" => await getExternalStorageDirectory(), |
There was a problem hiding this comment.
I think android and ios could be moved to some consts definition in top of the class.
|
Thank you for your effort, it looks good, but I have some questions. Check it please in your free time :) |
|
Yes, please, take your time to review it. I've also added a feature to show the |
|
LGTM! Thanks. |
This is a larger refactor, however, it's mostly cosmetic.
I've tested it and all looked OK, however, would be nice to throw another eye on it. :)