-
-
Notifications
You must be signed in to change notification settings - Fork 30
allow for directives to be added to the types #204
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
Conversation
Reviewer's Guide by SourceryThis pull request enhances the SQLAlchemy type mapper to allow for the addition of directives, improving its compatibility with GraphQL federation. The changes primarily affect the Updated class diagram for SQLAlchemy type mapperclassDiagram
class Mapper {
+type(model: Type[BaseModelType], make_interface: bool, use_federation: bool, directives: Union[Sequence[object], None] = ())
+convert(type_: Any) Any
}
note for Mapper "The 'type' method now accepts 'directives' to enhance GraphQL federation compatibility."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Thanks for adding the Here's a preview of the changelog: Added support for GraphQL directives in the SQLAlchemy type mapper, enabling better integration with GraphQL federation. Example usage: @mapper.type(Employee, directives=["@deprecated(reason: 'Use newEmployee instead')"])
class Employee:
pass |
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 @csechrist - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding documentation for the new 'directives' parameter in the type mapping function. This will help users understand how to utilize this new feature effectively.
- Please review and update the PR checklist. Ensuring all applicable items are checked will help maintain the project's quality standards.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Thank you for your contribution! I've reviewed your code and left one comment, please take a look when you can.
Additionally, it would be great if you could create new tests to ensure this solution remains stable in future updates.
I also believe this change needs to be added on our documentation. While I know our docs aren't perfect, adding a note to the README in Markdown will works for now. I think it could go right before the Limitations
section, what do you think?
Thanks again!
Sounds great! I will get tests added in and get the documentation updated this week! |
Hi, @csechrist , any updates? |
I pick up this PR and create new tests! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #204 +/- ##
==========================================
+ Coverage 85.51% 86.05% +0.54%
==========================================
Files 16 16
Lines 1629 1671 +42
Branches 139 136 -3
==========================================
+ Hits 1393 1438 +45
- Misses 173 174 +1
+ Partials 63 59 -4 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #204 will not alter performanceComparing Summary
|
@bellini666 @fruitymedley @patrick91 @erikwrede |
Thanks for contributing to Strawberry! 🎉 You've been invited to join You can also request a free sticker by filling this form: https://forms.gle/dmnfQUPoY5gZbVT67 And don't forget to join our discord server: https://strawberry.rocks/discord 🔥 |
Thank you so much, @csechrist ❤️ |
Description
Allow for directives to be added to the SQLAlchemy type mapper. This enables us to use it with GraphQL federation more effectively
Types of Changes
Issues Fixed or Closed by This PR
Checklist
Summary by Sourcery
Enhance the SQLAlchemy type mapper to support directives, facilitating better integration with GraphQL federation, and update pre-commit hooks to their latest versions.
Enhancements:
Build: