Skip to content

test [nfc]: Introduce "finish" helper for async checks #985

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

Merged
merged 2 commits into from
Oct 8, 2024

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Oct 5, 2024

This brings the file test/api/core_test.dart up to being clean against the unawaited_futures lint rule, toward #731.

It does so with a smaller diff, and simpler resulting code, than by adding await on each of the futures. For comparison:
#934 (comment)

@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Oct 5, 2024
@gnprice gnprice requested a review from PIG208 October 5, 2024 01:05
@gnprice gnprice mentioned this pull request Oct 5, 2024
@PIG208
Copy link
Member

PIG208 commented Oct 7, 2024

Looks like there is an issue with the dependency:

   info • The imported package 'test_api' isn't a dependency of the importing package • test/test_async.dart:1:8 • depend_on_referenced_packages

@gnprice
Copy link
Member Author

gnprice commented Oct 7, 2024

Oops, thanks. I had a commit for that but it was missing from the PR branch — I think I must have accidentally dropped it when moving the changes to an independent branch, after developing them on top of #934.

Should be fixed now.

@PIG208
Copy link
Member

PIG208 commented Oct 8, 2024

LGTM! Should be ready to merge then.

This brings the file test/api/core_test.dart up to being clean
against the `unawaited_futures` lint rule, toward zulip#731.

It does so with a smaller diff, and simpler resulting code,
than by adding `await` on each of the futures.  For comparison:
  zulip#934 (comment)
@gnprice gnprice merged commit 701763d into zulip:main Oct 8, 2024
1 check passed
@gnprice gnprice deleted the pr-test-async branch October 8, 2024 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer review PR ready for review by Zulip maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants