-
Notifications
You must be signed in to change notification settings - Fork 103
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
Sort agent IDs naturally #392
Conversation
@tvolkert I more followed my heart than actually tested this. How do I locally run app_dart? Are there tests for this somewhere I missed? |
https://github.com/flutter/cocoon/blob/master/app_dart/README.md#running-a-local-development-instance for how to run locally, but that also requires a local project clone... FYI, we can't deploy a new version of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -4,6 +4,7 @@ | |||
|
|||
import 'dart:async'; | |||
|
|||
import 'package:collection/collection.dart'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be added to our dependencies in pubspec.yaml
Holding, want to write tests. |
Future<Map<String, dynamic>> decodeHandlerBody() async { | ||
final Body body = await handler.get(); | ||
final List<Uint8List> bytes = await body.serialize().toList(); | ||
final String decodedBody = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tvolkert I messed with this for awhile. If there's a better way deserialize JSON from a Body I'm all ears.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The streams API leaves something to be desired... this will do the same thing with less code:
final String decodedBody = await utf8.decoder.bind(body.serialize()).join();
Or even:
return utf8.decoder.bind(body.serialize()).transform(json.decoder).single;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to the reader: normally I'd have suggested:
return body.serialize().transform(utf8.decoder).transform(json.decoder).single;
That looks much cleaner. But given my recent experience with dart-lang/sdk#36900, I've been shell-shocked into the code above.
|
||
setUp(() { | ||
config = FakeConfig(); | ||
buildStatusProvider = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: dartfmt -l 100
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes https://github.com/flutter/cocoon/issues/391.