-
-
Notifications
You must be signed in to change notification settings - Fork 251
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
Migrate readme to v3 #1188
Migrate readme to v3 #1188
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe changes involve modifications to the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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 (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/freezed/README.md (1)
255-264
: Alternative Assertion Approach Using a Private ConstructorProviding an alternative example that uses a private constructor (i.e.
Person._({required this.name}) : assert(name.isNotEmpty, 'name cannot be empty');
) offers flexibility for developers who prefer embedding assertions directly in the constructor. To avoid any potential confusion, consider adding a brief comment noting that this is an alternative to the@Assert
decorator approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/freezed/README.md
(13 hunks)
🔇 Additional comments (5)
packages/freezed/README.md (5)
47-47
: Index Update: Highlighting "Classic classes (NEW)"The addition of "(NEW)" in the Classic classes index clearly signals updated content to the reader. Ensure that any accompanying explanation in the section reinforces why these classes are highlighted as new.
245-249
: Enhanced Validation with @AssertAdding the
@Assert('name.isNotEmpty', 'name cannot be empty')
decorator in thePerson
factory constructor improves input validation and prevents empty names. This change adheres to good defensive programming practices.
784-786
: JSON Deserialization for MyResponseThe newly added
fromJson
factory method forMyResponse
:factory MyResponse.fromJson(Map<String, dynamic> json) => _$MyResponseFromJson(json);facilitates built-in JSON deserialization. Its concise arrow syntax improves clarity and aligns with best practices.
1187-1191
: Updated Example for Specific Model BehaviorThe example using
@Freezed()
to create an abstractPerson
class clearly demonstrates how to declare a model in line with the new v3 guidelines. This example is succinct and serves as a good reference.
1196-1201
: Customizing Generated Code with @freezed ParametersThe updated snippet:
@Freezed( // Disable the generation of copyWith/== copyWith: false, equal: false, ) abstract class Person with _$Person { ... }effectively shows how to alter the generated behavior for a model. Please verify that disabling
copyWith
and equality overrides aligns with your project requirements, as these settings affect immutability and object comparisons.
@snapsl, I've got some other changes in #1183. Would you like me to PR your fork so there's just one PR for @rrousselGit to review (because there's some conflicts right now, so it'll be slower to merge them in serial)? |
@lishaduck Thank you for the notice. |
packages/freezed/README.md
Outdated
@@ -44,7 +44,7 @@ to focus on the definition of your model. | |||
- [Extending classes](#extending-classes) | |||
- [Defining a mutable class instead of an immutable one](#defining-a-mutable-class-instead-of-an-immutable-one) | |||
- [Allowing the mutation of Lists/Maps/Sets](#allowing-the-mutation-of-listsmapssets) | |||
- [Classic classes](#classic-classes) | |||
- [Classic classes (NEW)](#classic-classes-new) |
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 don't like that (new) stuff.
If folks want to know what's new, there's the changelog
LGTM! |
I tried to change as little as possible.
Overall, I think a little restructuring of the readme file could be beneficial to make it more readable.
closes #1186
Summary by CodeRabbit
Summary by CodeRabbit
Documentation
Refactor
const
keywords for factory constructors, promoting immutability.fromJson
factory method in theMyResponse
class for improved JSON deserialization.