-
-
Notifications
You must be signed in to change notification settings - Fork 531
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
Fix mypy issue with async resolvers #3516
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3516 +/- ##
==========================================
- Coverage 96.53% 94.34% -2.19%
==========================================
Files 520 519 -1
Lines 33243 32183 -1060
Branches 5527 3683 -1844
==========================================
- Hits 32090 30363 -1727
- Misses 920 1538 +618
- Partials 233 282 +49 |
CodSpeed Performance ReportMerging #3516 will not alter performanceComparing Summary
|
Apollo Federation Subgraph Compatibility Results
Learn more: |
Thanks for adding the Here's a preview of the changelog: This release fixes an issue where mypy would complain when using a typed async Now the code will type check correctly. We also updated our test suite to make Here's the tweet text:
|
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.
Hey @patrick91 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 6 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 10 issues found
- 🟢 Complexity: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
From discord, looks like we introduced a regression for mypy in #3241
Not sure why our test suite was passing though, I think before merging we should figure that out (assuming this fixes it properly)I've updated how we test mypy to prevent similar issue in future