-
Notifications
You must be signed in to change notification settings - Fork 30
docs: add AI rules for Flutter and Dart #460
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 GuideAdds a new Claude AI rules document for Flutter/Dart to the repository, defining detailed coding, architecture, tooling, and UX guidelines for AI-generated Flutter code. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Dependency Reviewβ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
|
Claude finished @behnam-deriv's task ββ View job PR Review: Add AI rules for Flutter and DartI've completed my review of this PR that adds comprehensive AI rules for Flutter and Dart development. Overall, this is a well-structured and valuable documentation addition, but I've identified several issues that should be addressed. High Priority Issues1. Duplicate and Inconsistent Content (Lines 446-465)Location: There's a problematic section with duplicated content and incomplete sentences: ## UI Theming and Styling Code
* **Responsiveness:** Use `LayoutBuilder` or `MediaQuery` to create responsive
UIs.
* **Text:** Use `Theme.of(context).textTheme` for text styles.
* **Text Fields:** Configure `textCapitalization`, `keyboardType`, and
* **Responsiveness:** Use `LayoutBuilder` or `MediaQuery` to create responsive
UIs.
* **Text:** Use `Theme.of(context).textTheme` for text styles.
remote images.Issues:
Recommendation: Remove this entire section (lines 446-465) as the content is already covered in the "Material Theming Best Practices" section below, or restructure it with complete, non-duplicate content. 2. Broken Reference Link (Line 43)Location: See the [navigation guide](./navigation.md) for a detailed example using `go_router`.Issue: This references a Recommendation: Either remove this reference or clarify where the navigation guide can be found, or create the referenced file. 3. Inconsistent Tool References in "Interaction Guidelines"Location: The document references specific tools that may not be available in all AI contexts:
Issue: These tool references assume a specific AI environment/toolset that may not match Claude Code's actual capabilities. Claude Code uses standard bash commands like Recommendation: Either:
Medium Priority Issues4. Logging Package InconsistencyLocation: Conflict:
Issue: These are two different approaches to logging with different use cases. The document should clarify when to use each. Recommendation: Clarify that:
5. Potential Conflict with Project-Specific CLAUDE.mdLocation: Multiple sections The project's
Good news: Most guidelines align well! However, the new rules file is very generic and doesn't reference the specific Deriv Chart project context. Recommendation: Add a note at the top of
6. State Management Guidance May Not Fit Library ContextLocation: The extensive state management guidance focuses on app-level patterns (ValueNotifier, ChangeNotifier, Provider, MVVM), but this project is a charting library, not an application. Issue: The guidance is application-centric and may not be relevant for library contributors. Recommendation: Either:
Low Priority Issues7. Overly Prescriptive Design GuidanceLocation: Guidance like "Apply subtle noise texture to the main background" and "Multi-layered drop shadows create a strong sense of depth" is very design-specific and may not apply to all projects, especially a charting library. Recommendation: Consider making these suggestions rather than rules, or remove them as they're too prescriptive for a general-purpose guide. 8. Typo in Assets SectionLocation: * **Cached images:** For cached images, use NetworkImage a package likeIssue: Missing "with" or "or" - should be "use NetworkImage with a package like" or "use a package like" 9. Missing Comma in ExampleLocation: labelSmall: TextStyle(fontSize: 11.0, color: Colors.grey),Note: While this works, hardcoding Positive Aspectsβ
Comprehensive coverage of Flutter and Dart best practices SummaryThis is a valuable documentation addition, but requires refinement before merging: Must Fix (Blockers):
Should Fix (Important): Nice to Have: Recommendation: Request changes to address high-priority issues before merging. The document has strong foundations but needs cleanup to be production-ready. |
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 - I've found 3 issues, and left some high level feedback:
- In the 'UI Theming and Styling Code' section there are duplicated bullets for Responsiveness/Text and an incomplete 'Text Fields:' line ending with 'remote images.'βit would be good to clean this up for clarity and remove the repetition.
- The logging guidance is slightly inconsistent: earlier you recommend the
loggingpackage instead ofprint, while the later 'Logging' section focuses ondart:developer.log; consider explicitly explaining when to choose each so the rules are not ambiguous.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the 'UI Theming and Styling Code' section there are duplicated bullets for Responsiveness/Text and an incomplete 'Text Fields:' line ending with 'remote images.'βit would be good to clean this up for clarity and remove the repetition.
- The logging guidance is slightly inconsistent: earlier you recommend the `logging` package instead of `print`, while the later 'Logging' section focuses on `dart:developer.log`; consider explicitly explaining when to choose each so the rules are not ambiguous.
## Individual Comments
### Comment 1
<location> `.claude/rules/flutter-dart-rules.md:77-78` </location>
<code_context>
+ * Use `PascalCase` for classes, `camelCase` for
+ members/variables/functions/enums, and `snake_case` for files.
+* **Functions:**
+ * Functions short and with a single purpose (strive for less than 20 lines).
+* **Testing:** Write code with testing in mind. Use the `file`, `process`, and
+ `platform` packages, if appropriate, so you can inject in-memory and fake
</code_context>
<issue_to_address>
**issue (typo):** Clarify the functions guideline by adding the missing verb.
The wording is grammatically incomplete. Please rephrase, e.g. βKeep functions short and with a single purposeβ or βFunctions should be short and have a single purpose.β
```suggestion
* **Functions:**
* Keep functions short and with a single purpose (strive for less than 20 lines).
```
</issue_to_address>
### Comment 2
<location> `.claude/rules/flutter-dart-rules.md:426-427` </location>
<code_context>
+ Image.asset('assets/images/placeholder.png')
+ ```
+* **Network images:** Use NetworkImage for images loaded from the network.
+* **Cached images:** For cached images, use NetworkImage a package like
+ `cached_network_image`.
+* **Custom Icons:** Use `ImageIcon` to display an icon from an `ImageProvider`,
+ useful for custom icons not in the `Icons` class.
</code_context>
<issue_to_address>
**issue (typo):** Correct the grammar in the cached images guideline.
The phrase "use NetworkImage a package like `cached_network_image`" is missing a conjunction. Please change it to "use `NetworkImage` or a package like `cached_network_image`" and wrap `NetworkImage` in backticks for consistency with other API names.
```suggestion
* **Cached images:** For cached images, use `NetworkImage` or a package like
`cached_network_image`.
```
</issue_to_address>
### Comment 3
<location> `.claude/rules/flutter-dart-rules.md:169-172` </location>
<code_context>
+### State Management
+* **Built-in Solutions:** Prefer Flutter's built-in state management solutions.
+ Do not use a third-party package unless explicitly requested.
+* **Streams:** Use `Streams` and `StreamBuilder` for handling a sequence of
+ asynchronous events.
+* **Futures:** Use `Futures` and `FutureBuilder` for handling a single
+ asynchronous operation that will complete in the future.
+* **ValueNotifier:** Use `ValueNotifier` with `ValueListenableBuilder` for
</code_context>
<issue_to_address>
**nitpick (typo):** Consider using singular type names `Stream` and `Future` in backticks.
In the bullet points, the backticked identifiers are currently `Streams` and `Futures`. Since the actual Dart types are `Stream` and `Future`, please update the backticked names accordingly (e.g., "Use `Stream` and `StreamBuilder`", "Use `Future` and `FutureBuilder`") for accuracy and consistency with Dart API naming.
```suggestion
* **Streams:** Use `Stream` and `StreamBuilder` for handling a sequence of
asynchronous events.
* **Futures:** Use `Future` and `FutureBuilder` for handling a single
asynchronous operation that will complete in the future.
```
</issue_to_address>Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
|
Claude encountered an error ββ View job I'll analyze this and get back to you. |
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
|
Claude encountered an error ββ View job I'll analyze this and get back to you. |
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
|
Claude finished @behnam-deriv's task ββ View job PR Review: Add AI rules for Flutter and DartI've completed my review of this PR that adds comprehensive AI rules for Flutter and Dart development. The document is well-structured and covers important topics, but I've identified several issues that should be addressed before merging. High Priority Issues (Must Fix)1. Duplicate and Incomplete ContentLocation: There's a problematic section titled "UI Theming and Styling Code" with duplicated content and an incomplete sentence: * **Text Fields:** Configure `textCapitalization`, `keyboardType`, and
* **Responsiveness:** Use `LayoutBuilder` or `MediaQuery` to create responsiveIssues:
Recommendation: Remove lines 446-465 entirely, or rewrite this section with complete, non-duplicate content. 2. Broken Reference LinkLocation: See the [navigation guide](./navigation.md) for a detailed example using `go_router`.Issue: This references a Recommendation: Either remove this reference or create the referenced file, or update the link to point to an existing resource. 3. Inconsistent Tool ReferencesLocation: Multiple locations (lines 19, 21, 24, 47, 49, 326) The document references specific AI tools that may not match the actual environment:
Issue: These tool names suggest a specific AI environment/toolset. Claude Code uses standard bash commands like Recommendation: Replace tool references with actual command examples:
Medium Priority Issues (Should Fix)4. Conflicting Logging GuidanceLocation: Conflict:
Issue: These are two different approaches with different use cases, and the document presents them as if they're both the standard approach without clarifying when to use each. Recommendation: Clarify the distinction:
5. Type Name InconsistencyLocation: * **Streams:** Use `Streams` and `StreamBuilder`...
* **Futures:** Use `Futures` and `FutureBuilder`...Issue: The actual Dart types are Recommendation: Update to use correct type names:
(This was also flagged by Sourcery-ai) 6. Grammar Error in Cached ImagesLocation: * **Cached images:** For cached images, use NetworkImage a package like
`cached_network_image`.Issue: Missing conjunction - should be "use Recommendation: Fix to: "use (Also flagged by Sourcery-ai) 7. Incomplete Sentence in Functions GuidelineLocation: * **Functions:**
* Keep functions short and with a single purpose (strive for less than 20 lines).Issue: The original text reads "Functions short and with a single purpose" - missing a verb. Recommendation: Should read "Keep functions short..." or "Functions should be short..." (Also flagged by Sourcery-ai) Low Priority Issues (Nice to Have)8. Context Mismatch: App-Centric Guidance for a LibraryLocation: Issue: This project is a charting library, not an application. The extensive state management guidance (ValueNotifier, ChangeNotifier, Provider, MVVM) is very app-centric and may not be directly relevant for library contributors. Recommendation: Add a note clarifying that these rules are more relevant for applications using the library, or adjust the guidance to be more library-focused. 9. Overly Prescriptive Design GuidanceLocation: Guidance like:
Issue: This is very design-specific and opinionated. It may not apply to all projects, especially a technical charting library. Recommendation: Consider softening these as suggestions rather than rules, or remove them as they're too prescriptive for general-purpose guidance. 10. Hardcoded Color in Theme ExampleLocation: labelSmall: TextStyle(fontSize: 11.0, color: Colors.grey),Issue: While this code works, hardcoding Recommendation: Update example to use theme colors, or add a comment explaining this is just an example. 11. Potential Relationship Confusion with CLAUDE.mdObservation: The repository already has a Good news: Most guidelines align well between the two files:
Recommendation: Consider adding a note at the top of
Positive Aspects β
Summary and RecommendationThis is a valuable documentation addition that will help AI assistants generate better Flutter/Dart code. However, it requires refinement before merging. Blocking Issues (Must Fix Before Merge):
Important Issues (Should Fix): Recommendation: Request changes to address high-priority issues before merging. The document has strong foundations but needs cleanup to be production-ready. |
Clickup link:
Fixes issue: #
This PR contains the following changes:
Developers Note (Optional)
Pre-launch Checklist (For PR creator)
As a creator of this PR:
Reviewers
Pre-launch Checklist (For Reviewers)
As a reviewer I ensure that:
Pre-launch Checklist (For QA)
Pre-launch Checklist (For Maintainer)
Summary by Sourcery
Documentation: