Conversation
|
Warning Rate limit exceeded@rrousselGit has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 14 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces structural changes across the Freezed codebase. A required Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant FreezedGenerator
participant DataModel
Developer->>FreezedGenerator: Trigger code generation (@freezed)
FreezedGenerator->>DataModel: Create model with ConstructorDetails (isExternal flag)
DataModel-->>FreezedGenerator: Return model with updated structure
FreezedGenerator-->>Developer: Emit generated code
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/freezed/lib/src/freezed_generator.dart (1)
134-142: Remove unused commented-out code.
These lines are fully commented out and no longer enforced, which may cause confusion for future maintainers. If the restriction they enforce is no longer desired, consider fully removing this code to keep the codebase clean.- // if (constructor.factoryKeyword == null && - // (constructor.name?.lexeme != '_' || - // constructor.parameters.parameters.isNotEmpty)) { - // throw InvalidGenerationSourceError( - // 'Classes decorated with @freezed can only have a single non-factory' - // ', without parameters, and named MyClass._()', - // element: constructor.declaredElement, - // ); - // }packages/freezed/test/private_constructor_test.dart (1)
9-10: Use direct property access instead of cascade for clarity.
Althoughc..nametriggers property access, using the cascade here could be less intuitive. For clarity, considerc.name.- expect(() => c..name, throwsUnsupportedError); + expect(() => c.name, throwsUnsupportedError);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/_internal/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
packages/_internal/lib/models.dart(1 hunks)packages/_internal/lib/models.freezed.dart(15 hunks)packages/freezed/CHANGELOG.md(1 hunks)packages/freezed/README.md(2 hunks)packages/freezed/lib/src/freezed_generator.dart(3 hunks)packages/freezed/lib/src/models.dart(1 hunks)packages/freezed/lib/src/models.freezed.dart(33 hunks)packages/freezed/lib/src/templates/abstract_template.dart(0 hunks)packages/freezed/lib/src/templates/concrete_template.dart(0 hunks)packages/freezed/lib/src/templates/prototypes.dart(0 hunks)packages/freezed/test/generic_test.dart(0 hunks)packages/freezed/test/integration/multiple_constructors.dart(1 hunks)packages/freezed/test/integration/optional_maybe.dart(0 hunks)packages/freezed/test/map_test.dart(0 hunks)packages/freezed/test/multiple_constructors_test.dart(1 hunks)packages/freezed/test/optional_maybe_test.dart(0 hunks)packages/freezed/test/options_test.dart(1 hunks)packages/freezed/test/private_constructor_test.dart(1 hunks)packages/freezed/test/single_class_constructor_test.dart(0 hunks)packages/freezed/test/when_test.dart(0 hunks)packages/freezed_annotation/CHANGELOG.md(1 hunks)packages/freezed_annotation/lib/freezed_annotation.dart(0 hunks)packages/freezed_annotation/lib/freezed_annotation.g.dart(0 hunks)packages/freezed_annotation/test/freezed_test.dart(0 hunks)
💤 Files with no reviewable changes (12)
- packages/freezed/test/generic_test.dart
- packages/freezed/test/map_test.dart
- packages/freezed_annotation/lib/freezed_annotation.g.dart
- packages/freezed_annotation/test/freezed_test.dart
- packages/freezed/test/integration/optional_maybe.dart
- packages/freezed/lib/src/templates/concrete_template.dart
- packages/freezed/test/when_test.dart
- packages/freezed/test/single_class_constructor_test.dart
- packages/freezed/lib/src/templates/abstract_template.dart
- packages/freezed_annotation/lib/freezed_annotation.dart
- packages/freezed/test/optional_maybe_test.dart
- packages/freezed/lib/src/templates/prototypes.dart
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: freezed (packages/freezed_lint, master, get)
- GitHub Check: freezed (packages/freezed_annotation, master, get)
- GitHub Check: freezed (packages/freezed, master, get)
🔇 Additional comments (20)
packages/freezed/lib/src/freezed_generator.dart (2)
17-17: Re-export looks good.
The additional exported symbolsFreezedUnionCaseandFreezedUnionValueappear consistent with usage in this file and elsewhere.
380-380: Consider scenarios forisExternalusage.
The newisExternalproperty is always set tofalse. Verify if you plan to handle external constructors or update the property based on user inputs.packages/freezed/test/options_test.dart (6)
1-7: Imports appear valid.
No issues found regarding these imports. They align with the testing requirements.
9-23: Verify the no-issue test logic.
This test confirms thatoptional_maybe.dartcompiles without errors. It looks good for ensuring stable integration.
25-33: Confirm compile error check.
The approach to expect a compile error oncopyWithusage is effective for verifying that the feature is disabled.
35-50: Valid coverage for disablingtoStringand==/hash.
These tests confirm that the features are indeed disabled and behave as expected. Nicely done.
53-65: Thorough testing oftoJsondisabled behavior.
This ensures that trying to call a disabledtoJsontriggers a compile error. No issues found.
66-69: Verify forcedtoJsonresult.
The test effectively checks that a forcedtoJsonimplementation is generated correctly.packages/_internal/lib/models.dart (1)
45-45: LGTM! Added support for external constructors.The new
isExternalparameter inConstructorDetailsenables proper handling of external constructors.packages/freezed/lib/src/models.dart (1)
35-35: LGTM! Consistent with internal models.The changes maintain consistency with the internal models by adding the
isExternalparameter.packages/freezed/test/integration/multiple_constructors.dart (1)
107-107: LGTM! Enhanced type safety with sealed class.Making
NameConflictsealed ensures exhaustive pattern matching and better type safety by requiring all subclasses to be defined in the same file.packages/freezed/test/multiple_constructors_test.dart (1)
353-353: LGTM! Updated to use Dart's native pattern matching.The test now uses Dart's native pattern matching syntax with
switchexpressions, aligning with modern Dart practices.packages/_internal/lib/models.freezed.dart (3)
1-4: LGTM: Generated code header is properly maintained.The file correctly maintains the "GENERATED CODE" header with appropriate linter ignores.
241-242: LGTM: NewisExternalproperty added toConstructorDetails.The
isExternalproperty is properly added to the mixin with appropriate error handling.
772-790: LGTM:Dataclass properly updated to remove map/when functionality.The class structure has been simplified by removing map/when related properties while maintaining essential functionality.
packages/freezed/lib/src/models.freezed.dart (1)
26-28: LGTM: Improved documentation for copyWith methods.Clear and consistent documentation added for copyWith functionality.
packages/freezed_annotation/CHANGELOG.md (1)
1-4: LGTM: Clear documentation of breaking changes.The changelog properly documents the removal of
when/maprelated options as a breaking change in version 3.0.0.packages/freezed/CHANGELOG.md (1)
1-4: LGTM: Well-documented breaking change with rationale.The changelog clearly explains the removal of
map/whenand variants, providing the rationale that these features have been discouraged since Dart got pattern matching.packages/freezed/README.md (2)
9-9: LGTM! Enhanced package description.The updated description better reflects Freezed's capabilities by explicitly mentioning "tagged unions" and "nested classes".
419-419: LGTM! Improved technical accuracy.The clarification that the generated code will "extend/subclass" rather than "implement" the class is more precise and helps users better understand the inheritance mechanism.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/freezed/lib/src/freezed_generator.dart (1)
12-24: 🛠️ Refactor suggestionAdjust the fee structure to align with customer incentives.
The implementation of a flat $20 fee on discounted bills could negate the benefit of the discount, especially for smaller purchases or marginal loyalty tiers. This might lead to customer dissatisfaction, as the intent to reward loyalty paradoxically increases the bill.
Revise either the discount percentages or the flat fee application to better align with customer incentives.
Do you want me to generate a revised implementation that aligns better with customer expectations and loyalty rewards?
🧹 Nitpick comments (1)
packages/freezed/lib/src/freezed_generator.dart (1)
4-4: Reminder: Address the TODO comment.The TODO comment indicates that tests are missing for this function. Please ensure that the additional parameter change is thoroughly tested to confirm that it behaves as expected.
Do you want me to generate the unit testing code or open a new issue to track this task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/_internal/lib/models.dart(0 hunks)packages/_internal/lib/models.freezed.dart(1 hunks)packages/freezed/lib/src/freezed_generator.dart(1 hunks)packages/freezed/lib/src/models.dart(0 hunks)packages/freezed/lib/src/models.freezed.dart(21 hunks)
💤 Files with no reviewable changes (2)
- packages/freezed/lib/src/models.dart
- packages/_internal/lib/models.dart
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: freezed (packages/freezed_lint, master, get)
- GitHub Check: freezed (packages/freezed_annotation, master, get)
- GitHub Check: freezed (packages/freezed, master, get)
🔇 Additional comments (2)
packages/_internal/lib/models.freezed.dart (1)
1-1335: Generated file - no direct modifications needed.This is an auto-generated file by the Freezed package. Any changes should be made to the source files rather than modifying this file directly.
packages/freezed/lib/src/models.freezed.dart (1)
1-1335: Generated file - no direct modifications needed.This is an auto-generated file by the Freezed package. Any changes should be made to the source files rather than modifying this file directly.
1 participant
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests