docs(repo): add STYLE_GUIDE.md and TESTING.md adapted from Flutter's guides#2793
Open
xsahil03x wants to merge 14 commits into
Open
docs(repo): add STYLE_GUIDE.md and TESTING.md adapted from Flutter's guides#2793xsahil03x wants to merge 14 commits into
xsahil03x wants to merge 14 commits into
Conversation
Adds a canonical style guide at the repo root, adapted from https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md. Divergences from Flutter's guide (bare `// ignore` directives, `//` for private members, streams as the primary reactive primitive) are called out inline. CLAUDE.md now opens with a pointer directing AI agents to read STYLE_GUIDE.md before writing or reviewing code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR adds repository-wide guidance in ChangesRepository documentation guidance
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Verified 15 of 16 TODOs in the repo use bare `// TODO:` (no handle); one uses a category tag `// TODO(perf-migration):`. Flutter's `TODO(github-handle):` convention is not what this repo does. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_GUIDE.md `melos run format` wraps `dart format --set-exit-if-changed .` across every package with consistent settings; the guide now points contributors at the melos command they actually run, not the underlying binary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@STYLE_GUIDE.md`:
- Around line 186-192: The two fenced ASCII-tree diagrams in STYLE_GUIDE.md are
missing an explicit language tag, causing the markdownlint MD040 warning. Update
both fenced blocks by adding a tag such as text to the opening fence so the docs
remain lint-clean. Use the fenced diagram sections around the package tree
examples as the target for this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@STYLE_GUIDE.md`:
- Around line 1072-1074: Update the wording in the TODO guidance text to
capitalize “GitHub” so it reads “GitHub handle.” Locate the sentence describing
the optional short tag in the TODO format section and replace the lowercase
platform name while keeping the rest of the guidance unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Themes use theme_extensions_builder to generate copyWith/merge/lerp/equality from @ThemeExtensions annotations — the guide previously told contributors to hand-roll these. Also aligns with the repo's Flutter-style pattern of placing defaults in the widget implementation (nullable theme fields with null-coalescing resolution in build), not baked into StreamChatThemeData. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…CLAUDE Both docs referenced the project by name only; now link the file directly so contributors and agents can click through. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ction Verified against packages/stream_chat_flutter/lib/src/theme: component themes use bare @themeGen and don't extend ThemeExtension; the @ThemeExtensions(constructor: 'raw', ...) form is exclusive to the root StreamChatThemeData so it can plug into Material's ThemeData.extensions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
"Never skip without a linked issue" was overly strict — a skip because a test is flaky or slow shouldn't need a fabricated issue link. What matters is the reason being captured; a link is a bonus when there's a tracked issue. Softened to require a reason, encourage a link when applicable, and suggest filing an issue if the skip becomes long-lived. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Opening line said "Themes are ThemeExtensions" but the following steps clarify only the root is. Rephrased to name @themeGen upfront so the paragraph doesn't contradict the detailed steps beneath it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ests Adds a companion doc at the repo root covering how to write good tests (behavior-based test names, one behavior per test, extracting irrelevant setup, optimizing for comprehension). STYLE_GUIDE.md's Testing section now points to TESTING.md for the deeper guidance; STYLE_GUIDE keeps the tool/convention rules (mocktail, alchemist, self-containment). Examples are Stream-chat-flavoured rather than Flutter-widget-flavoured, and modernised (no `new`, no magic-number expectations that would contradict STYLE_GUIDE's "avoid magic numbers" rule). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- StreamMessageListController → StreamChannelListController (real class). - Reworked the state-transition example to use Message.copyWith on text (MessageState.sending doesn't exist; actual states are initial/outgoing/ completed/failed, and state is non-nullable so the `state: null` assert was bogus). - StreamReactionPicker → StreamMessageReactionPicker (actual class name). - Renamed the "Tapping a message" test to "Long-pressing" to match the gesture the example actually performs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add `text` language tag to the two ASCII-tree fenced blocks (layered package diagram, monorepo layout tree) so markdownlint MD040 stops warning on unlanguaged fences. - Capitalize "GitHub" in the TODO section's category-tag guidance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
xsahil03x
changed the title
Adds a contextual pointer near the test commands so agents writing tests hit the writing-effective-tests guidance directly, without needing to transit through STYLE_GUIDE.md first. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
Matches the format the equivalent section in stream-core-flutter's STYLE_GUIDE.md uses after its recent cleanup — a compact theme-file skeleton (annotation, mixin, InheritedTheme, of() lookup) plus a widget-consumption sample showing the null-coalescing default chain. The previous six-step numbered walkthrough was harder to model against than a copy-paste-worthy skeleton. No rule changes, just format. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
VelikovPetar
reviewed
Jul 1, 2026
|
|
||
| ## A word on designing APIs | ||
|
|
||
| Designing an API is an art. Like all forms of art, one learns by practicing. The best |
Contributor
There was a problem hiding this comment.
I find this section a bit weird, I am not sure if it contributes anything (except maybe the last sentence).
| ```text | ||
| stream_chat # Pure Dart, no Flutter dependency | ||
| └── stream_chat_persistence # Local disk cache using Drift (optional) | ||
| └── stream_chat_flutter_core # Flutter business logic, no UI |
Contributor
There was a problem hiding this comment.
I think this is not fully correct, stream_chat_flutter_core is not built on top of stream_chat_persistence
| mostly do the right thing but don't give the developer any way to adjust the results. | ||
| Predictability is reassuring. | ||
|
|
||
| ### Solve real problems by literally solving a real problem |
Contributor
There was a problem hiding this comment.
Not sure if this section is relevant - can we expect direct communication with customers here? (relevant for developers, but not for agents I would say)
| would be at the level that most developers interact with. Then, once that API is | ||
| cleanly designed, build the lower levels so the higher level can be layered atop. | ||
|
|
||
| Concretely: design the `StreamChatFlutter` widget or `Channel` method first, then the |
Contributor
There was a problem hiding this comment.
Do we have something like StreamChatFlutter? This should probably reference the package stream_chat_flutter
|
|
||
| ### Copyright and licensing | ||
|
|
||
| New source files should carry the standard header used elsewhere in the repo. Third- |
Contributor
There was a problem hiding this comment.
Hmm do we have a standard header that we apply to each file?
| melos run update:goldens | ||
| ``` | ||
|
|
||
| Regenerate goldens deliberately, in a separate commit from behavior changes, so |
Contributor
There was a problem hiding this comment.
Should we mention here the GitHub action? I am afraid this sentence might be misleading, and the agent will regenerate the Goldens locally
| the three). | ||
| 8. Read-only properties (other than `hashCode`). | ||
| 9. Operators (other than `==`). | ||
| 10. Methods (other than `toString` and `build`). |
Contributor
There was a problem hiding this comment.
Should we add some guidelines on how to group the methods? (public vs private, grouped by "feature"...)
| `translations_*.dart` file. | ||
| 3. Reference it via `StreamChatLocalizations.of(context)?.newString ?? 'fallback'` in | ||
| the widget. | ||
| 4. If the fallback isn't the same as the English translation, document why. |
Contributor
There was a problem hiding this comment.
I think we should add a reference to the test that requires updating as well - something I missed on my first localisation update.
|
|
||
| - Added `StreamChatClient.searchRoles` for searching channel roles. | ||
|
|
||
| 🐛 Fixed |
Contributor
There was a problem hiding this comment.
Small thing, but I think we should make sure to use the correct icon for the Fixed section: Sometimes we use :caterpillar:, sometimes :ladybug:.
|
|
||
| When writing a test, imagine the developer who will read it six months from now. | ||
| Anything you can do to help that reader understand what and why the test is checking | ||
| is worth doing. |
Contributor
There was a problem hiding this comment.
Mentioned it earlier: I think we should add a section about using test groups.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
STYLE_GUIDE.mdat the repo root — a canonical style guide adapted from Flutter's repo style guide, reshaped for this SDK.TESTING.mdat the repo root — a companion doc on how to write good tests adapted from Flutter's Writing-Effective-Tests. Complements STYLE_GUIDE.md's Testing section, which owns conventions (mocktail, alchemist, self-containment).CLAUDE.mdto open with a pointer telling AI agents to readSTYLE_GUIDE.mdbefore writing or reviewing code, and links the Chat SDK Design System Figma project directly (both docs).What's in the style guide
initStateinherited-lookup ban, named boolean parameters,file_names,directives_ordering, etc.). Every bullet links to its full section.stream_chatpure Dart.stream_core_flutter, generated-code policy.//not///).prefer_asserts_with_message), dispose lifecycle, equality boilerplates, dot shorthands, streams as the primary reactive primitive for real-time data, noInheritedWidgetlookup ininitState, early-return pattern, extensions on domain-typed collections OK but not on unconstrained types.mocktail,alchemistgolden tests. Points atTESTING.mdfor writing-style guidance.@themeGen+theme_extensions_buildercode generation (never hand-rollcopyWith/merge/lerp), Flutter-style defaults in the widget implementation (nullable theme fields, null-coalescing inbuild), rootStreamChatThemeDataas the soleThemeExtension.What's in TESTING.md
Four principles from Flutter's Writing-Effective-Tests, with Stream-chat-flavored examples:
Examples use real API surfaces (
StreamChannelListController,Channel.watch,StreamMessageComposer,Message.copyWith,StreamMessageReactionPicker).Divergences from Flutter's guide
Called out inline in the doc every time; the notable ones:
// ignore:directives are the repo style (verified: 38 bare vs 11 with rationale). Flutter's guide requires an explanatory comment; we do not.//, not///(verified: 51//vs 17///). Flutter uses///for "public-quality" private docs.ListenableoverStreaminside the framework; that reasoning doesn't apply to a real-time SDK.// TODO:matches repo practice (15 of 16), not Flutter'sTODO(handle):convention.Verified against the repo before merging
✅ Added,🔄 Changed,⚠️ Deprecated,🐛 Fixed,🔄 Internal / Non-breaking,🔒 Security) match what packages currently use.mocktail(the library actually in use), notmockito.theme_extensions_builder+@themeGen(bare) for component themes and the@ThemeExtensions(constructor: 'raw', ...)root-only pattern forStreamChatThemeData— verified againststream_chat_theme.dartandmessage_list_view_theme.dart.StringExtension/IntExtension/SafeCastExtensionand prescribes "no new ones on unconstrained common types".melos run format(the repo's canonical entrypoint), not rawdart format.StreamChannelListController,Message.copyWith,StreamMessageReactionPicker,Filter.equal,ConnectionStatus.connected,doInitialLoad,MockStreamChatClient).textlanguage tag on ASCII trees, "GitHub" capitalization).Test plan
TESTING.mdend-to-end — it's short (~190 lines).@themeGenwording match your day-to-day.🤖 Generated with Claude Code
Summary by CodeRabbit