fix: make unused resources fatal, and finish the feature that left two behind - #53
Merged
Merged
Conversation
…o behind CashPilot-android-xuc. Lint already DETECTED an orphaned resource but only as a warning, and the project's rule is that new lint ERRORS are fatal -- so detection worked and nothing acted on it. Raising the severity could not be done alone: two strings were already unused and would have failed the build immediately. The bead asked whether those meant the FEATURE was incomplete rather than the strings being junk. Checked both, and the answers differ: earnings_no_reading -- SUPERSEDED, deleted. The rule it was written for is implemented, just differently: FormatUtils.formatPlatformEarnings returns "—" for a null balance, so a per-app figure that was never read already renders as an em dash rather than $0.00. A text label would be a second, competing representation of the same state. earnings_shared_detail -- GENUINELY MISSING, now wired in. The dashboard shows "Shared across your devices" next to a per-app figure but never said what that means for the number directly above it. A user reasonably reads that number as what THIS phone earned, which is the misreading the label exists to prevent. It now renders the detail underneath, following the earnings_none_yet / _detail pair already established in EarningsCard. So half of it was an incomplete feature, which is why the bead was right to ask. THE GUARD IS PROVEN, not assumed. With a deliberately orphaned drawable: Error: The resource R.drawable.ic_control_orphan appears to be unused BUILD FAILED, exit 1 and with it removed, exit 0. Before this change the same file produced a warning and passed. Built and verified on the build host, since neither Mac has a working JVM: lintDebug clean, testDebugUnitTest passing, and verifyRoborazziDebug -- the exact task CI runs -- passing with no golden rewritten. Worth noting finalizeTestRoborazziDebug is SKIPPED under testDebugUnitTest alone and only runs under verifyRoborazziDebug, so the golden gate has to be invoked by name.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe pull request adds explanatory text for shared earnings, removes the obsolete ChangesEarnings display and lint enforcement
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
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
1 participant
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.
Closes CashPilot-android-xuc.
Lint already detected an orphaned resource — but only as a warning, and the project's rule is that new lint errors are fatal. So detection worked and nothing acted on it.
The severity change could not be made alone
Two strings were already unused and would have failed the build immediately. The bead asked whether that meant the feature was incomplete rather than the strings being junk. I checked both, and the answers differ:
earnings_no_reading— superseded, deleted.The rule it was written for is implemented, just differently:
FormatUtils.formatPlatformEarningsreturns—for a null balance, so a per-app figure that was never read already renders as an em dash rather than$0.00. A text label would be a second, competing representation of the same state.earnings_shared_detail— genuinely missing, now wired in.The dashboard shows "Shared across your devices" next to a per-app figure but never said what that means for the number directly above it. A user reasonably reads that number as what this phone earned — which is the misreading the label exists to prevent. It now renders the detail underneath, following the
earnings_none_yet/_detailpair already established inEarningsCard.So half of it was an incomplete feature, which is why the bead was right to ask rather than just deleting.
The guard is proven, not assumed
With a deliberately orphaned drawable:
With it removed: exit 0. Before this change, the same file produced a warning and passed.
Verification
Neither Mac has a working JVM, so everything ran on the build host:
lintDebug— clean, 0 errorstestDebugUnitTest— passingverifyRoborazziDebug— the exact task CI runs — passing, with no golden rewrittenWorth recording:
finalizeTestRoborazziDebugis SKIPPED undertestDebugUnitTestalone and only runs underverifyRoborazziDebug. The golden gate has to be invoked by name — running the unit tests is not enough to exercise it.Summary by CodeRabbit
New Features
Bug Fixes