Delinting future awaits round 1 #10760
Conversation
|
LGTM! |
| final String branch = _channel == 'alpha' ? 'alpha' : 'master'; | ||
| final DateTime remoteFrameworkCommitDate = DateTime.parse(await FlutterVersion.fetchRemoteFrameworkCommitDate(branch)); | ||
| // fire and forget since nothing subsequent depends on it. Hope there's a flush/sync mechanism. | ||
| // ignore: unawaited_futures |
There was a problem hiding this comment.
It looks ok in this case but as a general rule prefer to put the //ignore at the end of the offending line rather than before it
There was a problem hiding this comment.
May be a bug. Doesn't seem to actually work on multi-line expressions when it's like:
aFutureReturningFunction(
a,
b
); // ignore: unawaited_futures
There was a problem hiding this comment.
cc @leafpetersen but maybe try:
aFutureReturningFunction( // ignore: unawaited_futures
a,
b
);
| try { | ||
| final String branch = _channel == 'alpha' ? 'alpha' : 'master'; | ||
| final DateTime remoteFrameworkCommitDate = DateTime.parse(await FlutterVersion.fetchRemoteFrameworkCommitDate(branch)); | ||
| // fire and forget since nothing subsequent depends on it. Hope there's a flush/sync mechanism. |
There was a problem hiding this comment.
This hope seems optimistic...
cc @yjbanov did you mean to not wait for this future?
There was a problem hiding this comment.
Looks like a bug. I think we should await. I usually leave a comment if I'm dropping a Future on the floor intentionally.
| ); | ||
| await new Future<Null>.delayed(kPauseToLetUserReadTheMessage); | ||
| await Future.wait<Null>(<Future<Null>>[ | ||
| saveWarningStampFuture, |
There was a problem hiding this comment.
feel free to just inline the future-returning calls in the Future.wait call, fwiw.
There was a problem hiding this comment.
sent a follow up #10786
Do wish this syntax was more succinct/readable though
5 participants
#10754
Before I get too far, here's a sampling to make sure it's what we want.