Skip to content
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

Merged
merged 86 commits into from
Jul 17, 2024
Merged

Restructure modules and add documentation #3546

merged 86 commits into from
Jul 17, 2024

Conversation

patrick91
Copy link
Member

@patrick91 patrick91 commented Jun 22, 2024

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?

@botberry
Copy link
Member

botberry commented Jun 22, 2024

Apollo Federation Subgraph Compatibility Results

Federation 1 Support Federation 2 Support
_service🟢
@key (single)🟢
@key (multi)🟢
@key (composite)🟢
repeatable @key🟢
@requires🟢
@provides🟢
federated tracing🔲
@link🟢
@shareable🟢
@tag🟢
@override🟢
@inaccessible🟢
@composeDirective🟢
@interfaceObject🟢

Learn more:

Copy link

codecov bot commented Jun 22, 2024

Codecov Report

Attention: Patch coverage is 97.38134% with 16 lines in your changes missing coverage. Please review.

Project coverage is 96.57%. Comparing base (bea5cac) to head (846c0f8).

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     

Copy link

codspeed-hq bot commented Jun 22, 2024

CodSpeed Performance Report

Merging #3546 will not alter performance

Comparing feature/docstring (846c0f8) with main (bea5cac)

Summary

✅ 13 untouched benchmarks

@bellini666
Copy link
Member

@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?

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.

@patrick91
Copy link
Member Author

@bellini666 I meant to have types/directive.py (or similar) instead of
strawberry/directives/schema_directive.py as suggested in the issue

@bellini666
Copy link
Member

@bellini666 I meant to have types/directive.py (or similar) instead of strawberry/directives/schema_directive.py as suggested in the issue

Oh, I misunderstood the question haha

Totally ok by me to move it to types instead. Maybe types/directives is just over engineering =P

Copy link
Member

@DoctorJohn DoctorJohn left a 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.

strawberry/channels/testing.py Outdated Show resolved Hide resolved
strawberry/channels/testing.py Outdated Show resolved Hide resolved
strawberry/channels/testing.py Outdated Show resolved Hide resolved
strawberry/directive.py Show resolved Hide resolved
strawberry/exceptions/missing_dependencies.py Outdated Show resolved Hide resolved
strawberry/relay/types.py Outdated Show resolved Hide resolved
strawberry/relay/types.py Outdated Show resolved Hide resolved
strawberry/relay/types.py Outdated Show resolved Hide resolved
strawberry/relay/types.py Outdated Show resolved Hide resolved
strawberry/relay/types.py Outdated Show resolved Hide resolved
Copy link
Member

@DoctorJohn DoctorJohn left a 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/schema/schema.py Outdated Show resolved Hide resolved
strawberry/starlite/controller.py Outdated Show resolved Hide resolved
strawberry/test/client.py Outdated Show resolved Hide resolved
strawberry/types/auto.py Show resolved Hide resolved
tests/websockets/test_graphql_transport_ws.py Outdated Show resolved Hide resolved
tests/websockets/test_graphql_transport_ws.py Outdated Show resolved Hide resolved
tests/websockets/test_graphql_transport_ws.py Outdated Show resolved Hide resolved
tests/websockets/test_graphql_transport_ws.py Outdated Show resolved Hide resolved
tests/websockets/test_graphql_transport_ws.py Outdated Show resolved Hide resolved
Copy link
Member

@DoctorJohn DoctorJohn left a 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)

TWEET.md Outdated Show resolved Hide resolved
@patrick91
Copy link
Member Author

patrick91 commented Jul 17, 2024

time to merge

@patrick91 patrick91 merged commit b7a2077 into main Jul 17, 2024
64 of 65 checks passed
@patrick91 patrick91 deleted the feature/docstring branch July 17, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants