Skip to content

fix: remove sync_generator_adapter #22

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 1 commit into from
Feb 12, 2025
Merged

Conversation

sjinks
Copy link
Owner

@sjinks sjinks commented Feb 12, 2025

sync_generator_adapter was a mistake due to my misunderstanding of eager coroutines.

I thought an eager coroutine was completed synchronously from the caller's perspective and returned control to the caller when it was done. That was not true. An eager coroutine returns at the first suspending co_await.

Copy link

Copy link

codecov bot commented Feb 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.66%. Comparing base (b249844) to head (82ceffd).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #22      +/-   ##
==========================================
+ Coverage   84.56%   86.66%   +2.10%     
==========================================
  Files           6        5       -1     
  Lines         285      270      -15     
  Branches       54       53       -1     
==========================================
- Hits          241      234       -7     
  Misses          1        1              
+ Partials       43       35       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

Test Results

  5 files    5 suites   1m 1s ⏱️
 71 tests  71 ✅ 0 💤 0 ❌
355 runs  355 ✅ 0 💤 0 ❌

Results for commit 82ceffd.

@sjinks sjinks merged commit 2b50ab4 into master Feb 12, 2025
31 checks passed
@sjinks sjinks deleted the remove-sync-generator-adapter branch February 12, 2025 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant