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

Move Strawberry config to a typed dict #3365

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Checho3388
Copy link

Description

This MR aims to change Strawberry's schema config to be a typed dictionary. Then only one import would be required to instantiate the schema.

import strawberry
...
schema = strawberry.Schema(..., config={"auto_camel_case": False})

In the Schema class initialization the former dataclass is used to handle default values and validations. Then, my approach for this fix was not to remove the old dataclass.

UTs were updated accordingly.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@botberry
Copy link
Member

botberry commented Jan 29, 2024

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


Make schema config parameter a TypedDict

Here's the tweet text:

🆕 Release (next) is out! Thanks to @cheche3388 for the PR 👏

Get it here 👉 https://beta.strawberry.rocks/release/(next)

Copy link
Member

@patrick91 patrick91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! I'll take a proper look soon, but I was wonder if you're interested in writing a codemod for this too? 😊

We have one here: https://github.com/strawberry-graphql/strawberry/blob/main/strawberry/codemods/annotated_unions.py

Also I think we could keep the name of the dict the same, since it is mostly backwards compatible (dict(a=1) is almost the same as {"a": 1})

@Checho3388
Copy link
Author

Hi @patrick91 ,
Sure, I just wrote the codemod (at least an attempt to do it!). Please let me know your thoughts.
Regards

Copy link

codecov bot commented Feb 28, 2024

Codecov Report

Merging #3365 (042fd23) into main (a8f1a69) will decrease coverage by 1.09%.
Report is 12 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3365      +/-   ##
==========================================
- Coverage   96.40%   95.31%   -1.09%     
==========================================
  Files         498      500       +2     
  Lines       31120    31160      +40     
  Branches     3813     3809       -4     
==========================================
- Hits        30000    29701     -299     
- Misses        914     1250     +336     
- Partials      206      209       +3     

Copy link

codspeed-hq bot commented Feb 28, 2024

CodSpeed Performance Report

Merging #3365 will not alter performance

Comparing Checho3388:feature/move_strawberryconfig_to_a_typed_dict (042fd23) with main (a8f1a69)

Summary

✅ 12 untouched benchmarks

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.

3 participants