-
-
Notifications
You must be signed in to change notification settings - Fork 542
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
Restructure modules and add documentation #3546
Conversation
Apollo Federation Subgraph Compatibility Results
Learn more: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3546 +/- ##
==========================================
- Coverage 96.58% 96.57% -0.02%
==========================================
Files 524 525 +1
Lines 33632 33870 +238
Branches 5577 5587 +10
==========================================
+ Hits 32485 32711 +226
- Misses 912 922 +10
- Partials 235 237 +2 |
CodSpeed Performance ReportMerging #3546 will not alter performanceComparing Summary
|
IMO yes. I mean, unless we think that the way it is now is totally ok and we are never going to have to refactor that, I'd say better to do now and have a single "huge breaking" release than having to have another one later. |
@bellini666 I meant to have types/directive.py (or similar) instead of |
Oh, I misunderstood the question haha Totally ok by me to move it to |
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.
Awesome work @patrick91, loving it. I left a bunch of nits and at least three non-nits. Have a look :) I'm halfway trough the files, but need to get lunch now, I'll finish the rest later.
Co-authored-by: Jonathan Ehwald <[email protected]>
for more information, see https://pre-commit.ci
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.
Second half done 🎉 Still excellent changes. Most of these comments are nits again, some non-nits here and there. (Feel free to ignore all the adjustments made to test docsstrings. I couldn't resist though).
strawberry/subscriptions/protocols/graphql_transport_ws/handlers.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Jonathan Ehwald <[email protected]>
Co-authored-by: Jonathan Ehwald <[email protected]>
Co-authored-by: Jonathan Ehwald <[email protected]>
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.
Awesome work @patrick91! Looks good to me now. (note the typo in TWEET.md)
Co-authored-by: Jonathan Ehwald <[email protected]>
See #3542
strawberry.auto
->strawberry.types.auto
strawberry.private
->strawberry.types.private
strawberry.enum
->strawberry.types.enum
strawberry.union
->strawberry.types.union
strawberry.lazy_type
->strawberry.types.lazy
strawberry.unset
->strawberry.types.unset
strawberry.custom_scalar
->strawberry.types.scalar
strawberry.object_type
->strawberry.types.object_type
strawberry.type
->strawberry.types.type
strawberry.field
->strawberry.types.fields.field
strawberry.mutation
->strawberry.types.fields.mutation
strawberry.arguments
->strawberry.types.arguments
@bellini666 I've only added the ones that will go into the types module for now, I wonder if we should also move things like directives to the types module?