-
-
Couldn't load subscription status.
- Fork 596
Resolve TODO: Replace EnumDefinition with StrawberryEnum #3999
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's GuideThis PR systematically replaces the legacy EnumDefinition class and related _enum_definition attributes with the new StrawberryEnum construct, updating all core schema conversion paths, type checks, code generation, and tests to reference StrawberryEnum and strawberry_definition, while preserving backward compatibility through deprecation aliases and warnings. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3999 +/- ##
=======================================
Coverage 94.39% 94.39%
=======================================
Files 534 534
Lines 34834 34856 +22
Branches 1831 1831
=======================================
+ Hits 32881 32904 +23
Misses 1657 1657
+ Partials 296 295 -1 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #3999 will not alter performanceComparing Summary
|
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.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `RELEASE.md:3` </location>
<code_context>
+Release type: patch
+
+Resolve TODO's about EnumDefinition by renaming it to StrawberryEnum
</code_context>
<issue_to_address>
**suggestion (typo):** Consider changing "TODO's" to "TODOs" for correct pluralization.
The correct plural is "TODOs" without the apostrophe.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Thanks for adding the Here's a preview of the changelog: Deprecation Alert
Other Changes
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.
Greptile Summary
This PR performs a systematic codebase-wide refactoring that renames the EnumDefinition class to StrawberryEnum and updates all references throughout the Strawberry GraphQL library. The change affects 17 files across core modules, tests, and integrations, resolving existing TODO comments that had explicitly marked this renaming as planned technical debt.
The refactoring touches several key areas of the codebase:
- Core enum handling: The main enum class in
strawberry/types/enum.pyis renamed fromEnumDefinitiontoStrawberryEnum, with the__all__export list updated accordingly - Schema system: Updates to schema converters, name converters, and type checking logic to use the new class name
- Code generation: Query codegen and federation modules updated to properly handle the renamed enum type
- Type system: Updates to argument handling, annotation processing, and printer logic
- Integrations: Pydantic integration updated to work with the new enum class name
- Test suite: Comprehensive updates to test assertions and mock object creation
The change maintains complete backward compatibility since StrawberryEnum provides the identical interface and functionality as EnumDefinition. The renaming aligns with Strawberry's naming conventions where core types are prefixed with 'Strawberry' (similar to StrawberryType, StrawberryUnion, etc.). All functionality remains exactly the same - this is purely a naming standardization effort that removes deprecated references and improves code consistency across the library.
Confidence score: 5/5
- This PR is extremely safe to merge with no risk of breaking functionality
- Score reflects that this is a systematic renaming with identical interfaces and comprehensive coverage across all affected files
- No files require special attention as the changes are mechanical and maintain exact same behavior
19 files reviewed, no comments
…instead of _enum_definition
for more information, see https://pre-commit.ci
… in __strawberry_definition__
for more information, see https://pre-commit.ci
…berryObjectDefinition
for more information, see https://pre-commit.ci
| values.append(value) | ||
|
|
||
| cls._enum_definition = EnumDefinition( # type: ignore | ||
| cls.__strawberry_definition__ = StrawberryEnum( # type: ignore |
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.
should we also keep a deprecated ._enum_definiton for compatibility?
You can use this DeprecatedDescriptor, like we have for the old _type_definition
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.
Agree, I'll do it!
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.
Updated!
|
|
||
| @dataclasses.dataclass | ||
| class EnumDefinition(StrawberryType): | ||
| class StrawberryEnum(StrawberryType): |
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.
Like my comment below, we also need to expose the deprecated version of this
Like in https://github.com/strawberry-graphql/strawberry/blob/main/strawberry/types/base.py#L456-L456
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.
I agree, updating now!
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.
Updated, @bellini666 , I use the same implementation we done in this PR #2836 .
Please let me now if you see a better way to do it!
RELEASE.md
Outdated
| @@ -0,0 +1,3 @@ | |||
| Release type: patch | |||
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.
Would this be a minor? It does more than just fixing, it also changes variable names
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.
I agree 100% with you, also will add a deprecated info on the release
|
Already fixing this CI errors. |
|
Ready to review @bellini666 |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
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.
…ion to make ruff pre-commit happy
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.
Greptile Overview
Greptile Summary
This PR successfully completes a systematic refactoring that renames EnumDefinition to StrawberryEnum throughout the codebase, resolving TODOs in strawberry/schema/schema_converter.py at lines 872 and 885.
Key Changes
- Core renaming:
EnumDefinitionclass renamed toStrawberryEnuminstrawberry/types/enum.py - Attribute migration: All enum definitions now use
__strawberry_definition__instead of_enum_definition - Backward compatibility: Old
_enum_definitionattribute maintained viaDeprecatedDescriptorwith warnings - Type alias:
EnumDefinitionpreserved as a deprecated alias toStrawberryEnumfor imports - Comprehensive updates: All type hints, imports, and isinstance checks updated across 28 files
- Updated TODO comments: Removed resolved TODO comments from schema converter
Impact
- Standardizes naming convention to match other Strawberry types (e.g.,
StrawberryObjectDefinition) - Maintains full backward compatibility with existing code
- All tests updated to use new patterns
- Documentation added in RELEASE.md
Confidence Score: 5/5
- This PR is safe to merge with minimal risk
- The refactoring is comprehensive, systematic, and includes excellent backward compatibility. The deprecated descriptor pattern ensures existing code continues working while warning users. All 28 files are consistently updated, tests are thorough, and the changes follow established patterns in the codebase.
- No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| strawberry/types/enum.py | 5/5 | Renamed EnumDefinition to StrawberryEnum, added deprecation support for _enum_definition, and created backward compatibility alias |
| strawberry/schema/schema_converter.py | 5/5 | Updated all references from EnumDefinition to StrawberryEnum and resolved TODO comments |
| strawberry/schema/compat.py | 5/5 | Updated is_enum() function to check for StrawberryEnum using __strawberry_definition__ attribute |
| strawberry/types/base.py | 5/5 | Updated type checking logic to use StrawberryEnum and __strawberry_definition__ instead of _enum_definition |
| strawberry/utils/deprecations.py | 5/5 | Added _ENUM_DEFINITION deprecation message for backward compatibility |
| tests/test_deprecations.py | 5/5 | Added comprehensive tests for _enum_definition deprecation and EnumDefinition alias |
| strawberry/types/arguments.py | 5/5 | Replaced EnumDefinition imports and type checks with StrawberryEnum, updated __strawberry_definition__ usage |
| RELEASE.md | 5/5 | Added release notes documenting the EnumDefinition to StrawberryEnum rename and deprecation |
Sequence Diagram
sequenceDiagram
participant Dev as Developer
participant Enum as @strawberry.enum
participant Process as _process_enum()
participant Def as StrawberryEnum
participant Depr as DeprecatedDescriptor
participant Code as User Code
Dev->>Enum: Decorates Enum class
Enum->>Process: _process_enum(cls)
Process->>Def: Create StrawberryEnum instance
Process->>Code: Set cls.__strawberry_definition__
Process->>Depr: Create DeprecatedDescriptor
Depr->>Code: Inject _enum_definition property
Note over Code,Depr: New pattern (recommended)
Code->>Def: Access via __strawberry_definition__
Def-->>Code: Returns StrawberryEnum
Note over Code,Depr: Old pattern (deprecated)
Code->>Depr: Access via _enum_definition
Depr->>Depr: Emit deprecation warning
Depr-->>Code: Returns same StrawberryEnum
Note over Dev,Code: Type checking in schema conversion
Code->>Code: isinstance(type_, StrawberryEnum)
Code->>Code: type_.__strawberry_definition__
28 files reviewed, no comments
|
Ready for review again @bellini666 |

Description
Replace
EnumDefinitionwithStrawberryEnumTypes of Changes
Issues Fixed or Closed by This PR
Checklist
Summary by Sourcery
Replace the legacy EnumDefinition with StrawberryEnum throughout the core codebase, ensuring all conversions, codegen paths, and tests are updated to use the new class name.
Enhancements:
Documentation:
Summary by Sourcery
Migrate all enum-related logic to use the new StrawberryEnum class, remove the legacy EnumDefinition, and introduce deprecation aliases and warnings for backward compatibility
Enhancements:
Documentation:
Tests: