-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add documentation about Fail fast strategy in android #2697
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
📝 WalkthroughWalkthroughThe documentation for Android best practices was updated with a new "Fail fast" section, detailing strategies for early error detection and handling. Additionally, the linter documentation was expanded to clarify the use and organization of custom lint rules, including an example of a custom rule for missing serialization annotations. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant App (Debug Build)
participant App (Production Build)
participant FailFast API
participant Logger
Developer->>App (Debug Build): Runs code with potential error
App (Debug Build)->>FailFast API: Error encountered
FailFast API->>App (Debug Build): Crash app, show error message
Developer->>App (Production Build): Runs code with potential error
App (Production Build)->>FailFast API: Error encountered
FailFast API->>Logger: Log error
FailFast API->>App (Production Build): Return fallback value
sequenceDiagram
participant Developer
participant Lint Module
participant Custom Rule (MissingSerializableAnnotationIssue)
Developer->>Lint Module: Runs lint checks
Lint Module->>Custom Rule: Analyze code for @Serializable
Custom Rule->>Lint Module: Report missing annotation issues
Lint Module->>Developer: Show lint warnings/errors
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (2)
docs/android/linter.md (1)
144-147
: Clarify or link the:lint
module
Mentioning the dedicated Gradle module is great—consider adding a hyperlink to the module’s directory in the repo for quick navigation (e.g., “see the :lint module on GitHub”).docs/android/best_practices.md (1)
185-187
: Refine wording for formality
Consider replacing “identify, debug, and fix issues” with “identify, troubleshoot, and resolve issues” for a more formal tone.🧰 Tools
🪛 LanguageTool
[style] ~186-~186: Consider using a different verb for a more formal wording.
Context: ...make it easier to identify, debug, and fix issues before they reach production. W...(FIX_RESOLVE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
docs/android/best_practices.md
(1 hunks)docs/android/linter.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/android/best_practices.md
[style] ~149-~149: Consider a different adjective to strengthen your wording.
Context: ...rashes, silently ignoring them can hide deeper issues and make debugging more difficul...
(DEEP_PROFOUND)
[style] ~186-~186: Consider using a different verb for a more formal wording.
Context: ...make it easier to identify, debug, and fix issues before they reach production. W...
(FIX_RESOLVE)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - developers-home-assistant
- GitHub Check: Header rules - developers-home-assistant
- GitHub Check: Pages changed - developers-home-assistant
🔇 Additional comments (3)
docs/android/linter.md (2)
142-143
: Nice addition: call to contribute custom rules
Encouraging contributions here aligns well with the project’s philosophy of enforcing domain-specific patterns early.
148-148
: Good concrete example
TheMissingSerializableAnnotationIssue
bullet clearly communicates the rule’s purpose and ties back to serialization best practices.docs/android/best_practices.md (1)
120-124
: Approve new “Fail fast” section—verify navigation
The new section on fail-fast strategies fills an important gap. Ensure that this heading appears in the Android docs sidebar or table of contents so readers can find it easily.
Co-authored-by: Martin Hjelmare <[email protected]>
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.
After thinking how I would respond if someone submitted the examples in a PR, a few suggestions for the code and to make it easier to understand without looking at the FailFast
functions first.
Co-authored-by: Joris Pelgröm <[email protected]>
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
♻️ Duplicate comments (1)
docs/android/best_practices.md (1)
169-184
: Fix grammar and align error message
In theFailFast.failOnCatch
example, update the message to match the earlierTimber.w
log and correct the tense (e.g.,"External third party threw an error. Current state = …"
).
🧹 Nitpick comments (4)
docs/android/best_practices.md (4)
120-124
: Prefer stronger phrasing for compile-time checks
Consider updating “Always aim to catch errors at build time rather than at runtime” to “Opt for compile-time checks (e.g., via Kotlin compiler plugins or custom lint rules) whenever possible” to strengthen the recommendation.
147-168
: Catch specific exceptions
CatchingException
can mask unexpected errors. Consider catching only the exceptions you expect (e.g.,IOException
or a custom API exception) to avoid hiding programming issues.🧰 Tools
🪛 LanguageTool
[style] ~149-~149: Consider a different adjective to strengthen your wording.
Context: ...rashes, silently ignoring them can hide deeper issues and make debugging more difficul...(DEEP_PROFOUND)
186-188
: Use “resolve” instead of “fix”
Replace “debug, and fix issues” with “debug, and resolve issues” for a more formal tone.🧰 Tools
🪛 LanguageTool
[style] ~187-~187: Consider using a different verb for a more formal wording.
Context: ...make it easier to identify, debug, and fix issues before they reach production. W...(FIX_RESOLVE)
191-206
: Remove hard tabs in log block
The sample log contains hard tab characters that markdownlint flags. Convert them to spaces for consistent formatting.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
205-205: Hard tabs
Column: 101(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
docs/android/best_practices.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/android/best_practices.md
[style] ~149-~149: Consider a different adjective to strengthen your wording.
Context: ...rashes, silently ignoring them can hide deeper issues and make debugging more difficul...
(DEEP_PROFOUND)
[style] ~187-~187: Consider using a different verb for a more formal wording.
Context: ...make it easier to identify, debug, and fix issues before they reach production. W...
(FIX_RESOLVE)
🪛 markdownlint-cli2 (0.17.2)
docs/android/best_practices.md
205-205: Hard tabs
Column: 101
(MD010, no-hard-tabs)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - developers-home-assistant
- GitHub Check: Header rules - developers-home-assistant
- GitHub Check: Pages changed - developers-home-assistant
🔇 Additional comments (1)
docs/android/best_practices.md (1)
125-145
: Good Kotlin compiler example
The sealedwhen
snippet correctly demonstrates exhaustive checks without anelse
branch and is clear for readers.
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 (4)
docs/android/best_practices.md (4)
121-121
: Section header casing consistency
The heading## Fail fast
should match the title‐casing style of other sections. Consider renaming to## Fail Fast
or## Fail-Fast
for consistency.
147-151
: Strengthen exception-handling wording
The phrase“silently ignoring them can hide deeper issues and make debugging more difficult”
could be more impactful. For example:- silently ignoring them can hide deeper issues and make debugging more difficult + silently ignoring them can obscure root causes and hamper effective debugging🧰 Tools
🪛 LanguageTool
[style] ~149-~149: Consider a different adjective to strengthen your wording.
Context: ...rashes, silently ignoring them can hide deeper issues and make debugging more difficul...(DEEP_PROFOUND)
189-191
: Formal tone for “fix”
For a more formal tone, consider replacing “fix” with “resolve”:- debug, and fix issues before they reach production. + debug, and resolve issues before they reach production.🧰 Tools
🪛 LanguageTool
[style] ~190-~190: Consider using a different verb for a more formal wording.
Context: ...make it easier to identify, debug, and fix issues before they reach production. W...(FIX_RESOLVE)
194-209
: Remove hard tabs from log snippet
The log block contains hard-tab characters (e.g., line 208), which violates MD010. Replace leading tabs with spaces:- 2025-06-12 10:53:20.841 29743-29743 CrashFailFastHandler … + 2025-06-12 10:53:20.841 29743-29743 CrashFailFastHandler …🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
208-208: Hard tabs
Column: 101(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
docs/android/best_practices.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/android/best_practices.md
[style] ~149-~149: Consider a different adjective to strengthen your wording.
Context: ...rashes, silently ignoring them can hide deeper issues and make debugging more difficul...
(DEEP_PROFOUND)
[style] ~190-~190: Consider using a different verb for a more formal wording.
Context: ...make it easier to identify, debug, and fix issues before they reach production. W...
(FIX_RESOLVE)
🪛 markdownlint-cli2 (0.17.2)
docs/android/best_practices.md
208-208: Hard tabs
Column: 101
(MD010, no-hard-tabs)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - developers-home-assistant
- GitHub Check: Header rules - developers-home-assistant
- GitHub Check: Pages changed - developers-home-assistant
🔇 Additional comments (4)
docs/android/best_practices.md (4)
122-124
: Intro paragraph is clear
This introduction effectively communicates the importance of catching errors early and links to the lint documentation for compile-time checks.
127-135
: Kotlin compiler example is accurate
The sealed-interfacewhen
example correctly illustrates exhaustive checks with noelse
branch.
153-164
: Logging example is solid
This snippet correctly shows catching the exception, logging with context, and providing a fallback.
174-187
: FailFast API usage is clear
TheFailFast.failOnCatch
example succinctly demonstrates crash-in-debug vs. fallback-in-prod behavior.
Proposed change
Add a new section in
best practices
for Android about the newFailFast
API. It also add more details in the section about writing custom lint rules by adding the current custom rules that we have.Type of change
Checklist
Additional information
Summary by CodeRabbit