Feature: Latest launch #14
Conversation
|
I implemented the suggestions. |
| @@ -93,4 +107,24 @@ class SpaceXApiClient { | |||
| throw JsonDecodeException(); | |||
| } | |||
| } | |||
|
|
|||
| Future<Map<String, dynamic>> _getOne(Uri uri) async { | |||
There was a problem hiding this comment.
| Future<Map<String, dynamic>> _getOne(Uri uri) async { | |
| Future<Map<String, dynamic>> _get(Uri uri) async { |
There was a problem hiding this comment.
_get is already defined for a "List" return. _getOne only returns an item.
There was a problem hiding this comment.
An alternative might be _getMany for multiple results and _get for single ones, but I'm fine either way :)
@felangel Wdyt?
Unresolving for the time being for visibility 👀
|
@felangel It's done |
There was a problem hiding this comment.
First off, thanks for these changes!
I just have a couple styling comments. Once those are implemented (or otherwise resolved), I'll have another look at the app in runtime.
I'll let the CI run in the meantime so you can work on potential failures. 👍🏻
Great job!
| launches: | ||
| (json['launches'] as List<dynamic>).map((e) => e as String).toList(), |
There was a problem hiding this comment.
This will most likely fail the flutter format step 👀
| @@ -93,4 +107,24 @@ class SpaceXApiClient { | |||
| throw JsonDecodeException(); | |||
| } | |||
| } | |||
|
|
|||
| Future<Map<String, dynamic>> _getOne(Uri uri) async { | |||
There was a problem hiding this comment.
An alternative might be _getMany for multiple results and _get for single ones, but I'm fine either way :)
@felangel Wdyt?
Unresolving for the time being for visibility 👀
There was a problem hiding this comment.
I'm not sure if that's right. The stringify getter in the Equatable package is set to null.
@alefl10 @jeroen-meijer
There was a problem hiding this comment.
I've added some new tests, because code coverage is still too low. I looked the lcov results on my machine, and sometimes I don't understand why it did not cover certain code places. Furthermore, I also commented out failed tests in launches_page_test.dart so may have a look what I tried.
|
@jeroen-meijer @felangel Can someone please start the workflow? 😃 |
| linter: | ||
| rules: | ||
| public_member_api_docs: false | ||
| avoid_print: false |
There was a problem hiding this comment.
rather than disabling this for the whole package can we just ignore it from the example?
There was a problem hiding this comment.
What do you mean exactly? I added this file like in the rocket_repository 😅
| /// Throws a [LaunchException] if an error occurs. | ||
| Future<Launch> fetchLatestLaunch() { | ||
| try { | ||
| return _spaceXApiClient.fetchLatestLaunch(); |
There was a problem hiding this comment.
| return _spaceXApiClient.fetchLatestLaunch(); | |
| return await _spaceXApiClient.fetchLatestLaunch(); |
I believe we'd need to await the future in order to catch any exceptions.
There was a problem hiding this comment.
It only awaits the future in the presentation layer, like in the "launches_cubit". If this is true for catching any exceptions, we also have to change it, e.g. in the rocket_repository too.
Description
Just added the latest launch.
Type of Change