Skip to content
This repository was archived by the owner on Nov 20, 2024. It is now read-only.

stop discarding futures #3455

Merged
merged 1 commit into from
Jun 14, 2022
Merged

stop discarding futures #3455

merged 1 commit into from
Jun 14, 2022

Conversation

pq
Copy link
Contributor

@pq pq commented Jun 14, 2022

@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 96.321% when pulling 5865add on stop_discarding_ftures into a91b233 on master.

@pq pq merged commit 045f2fd into master Jun 14, 2022
@pq pq deleted the stop_discarding_ftures branch June 14, 2022 20:05
@@ -20,10 +20,10 @@ import 'package:path/path.dart' as path;

import 'rules/visit_registered_nodes.dart';

void main() {
Future<void> main() async {
Copy link
Contributor

@albertms10 albertms10 Jun 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per #3229, we may drop the Future annotation in favor of void in top-level main functions.

WDYT, @pq? If you want, I can open a new PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relaxing avoid_void_async to allow this idiom does sort of suggest this Future return type annotation is gratuitous. Interestingly, this is what the analyzer ADD_ASYNC quick-fix produces. Maybe we should consider updating it to not do this for main?

/cc @bwilkerson

As for a cleanup PR, @albertms10, sure thing.

Thanks for the feedback!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea how common that idiom is, but if it's common enough then it would make sense to support it. Not sure who ought to be looped in to the conversation.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants