Repository navigation
refactor: fix the causes behind fixable suppressions - #257
Conversation
startActivity documents ActivityNotFoundException and SecurityException as its failure modes; catching every Exception also hid programming errors. Other exceptions now propagate, which the new test pins.
…ters configureItemSwipeAnimator hands the view holder and action state to its callbacks although the icon tab needs neither; lambdas with an underscore parameter express that without suppressing UNUSED_PARAMETER.
onCreate and onStopListening only called super, so they needed redundantOverride and EmptyMethod suppressions without changing behavior.
The about-screen app icon sits next to the app name, and the settings dialog separators carry no information, so TalkBack skips them instead of the layouts suppressing ContentDescription.
The SearchView separator label moves to a non-translatable string like the other widget-name labels. The icon list image repeats the name shown beside it, so it is decorative instead of announcing a hardcoded "Icon".
ACTION_APP_LOCALE_SETTINGS exists from API 33 and the only caller is gated on TIRAMISU; @RequiresApi states that contract for lint instead of suppressing InlinedApi.
…cast filterIsInstance yields the typed set directly, so the UNCHECKED_CAST suppression goes away.
…ools detekt does not analyze the release source set, so the suppression never applied.
Starting an external activity throws an open-ended set of exceptions; a narrowed catch turns a failed link into a crash instead of the error toast.
Plan limit reachedYou've used all Starter plan credits for this billing period (70 limit), so this pull request was not reviewed ( Upgrade to a higher plan or wait until your next billing period: See plans on the billing page |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe changes add lifecycle logging to ChangesUI and Kotlin updates
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to No material merge-blocking behavior change was established; the locale-settings action remains unavailable below API 33. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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. A rabbit logs the tile’s next call, Comment |
 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The sample demos the TileService lifecycle, so both callbacks are overridden again with a Log.d line. The log call gives each override a body beyond super, so neither needs a redundantOverride or EmptyMethod suppression. A Robolectric test pins the logged messages.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| override fun onCreate() { | ||
| // Called by the system when the service is first created. | ||
| super.onCreate() | ||
| Log.d(TAG, "onCreate") |
There was a problem hiding this comment.
onCreate and onStopListening overrides only call super and write debug logs. Because these methods run in the production tile lifecycle, they add log output and maintenance without doing any service work. Removing the overrides would avoid the suppressions without adding those side effects.
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/main/java/de/lemke/oneuisample/ui/QSTileService.kt
Line: 34
Comment:
**Unnecessary lifecycle logging** The restored `onCreate` and `onStopListening` overrides only call `super` and write debug logs. Because these methods run in the production tile lifecycle, they add log output and maintenance without doing any service work. Removing the overrides would avoid the suppressions without adding those side effects.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Fixes the causes behind suppressions that had a real fix, and removes those suppressions.
3c02c45TabIconsFragment passes swipe callbacks as lambdas with underscore parameters, soUNUSED_PARAMETERgoes away.dcf660bQSTileService drops the super-onlyonCreateandonStopListeningoverrides, soredundantOverrideandEmptyMethodgo away.54cacd2Decorative images (about-screen app icon, settings dialog separators) useimportantForAccessibility="no"instead of suppressingContentDescription.2cd99f4The SearchView separator label moves to a non-translatable string. The icon list image becomes decorative instead of announcing a hardcoded "Icon". BothHardcodedTextsuppressions go away.4e65cfdopenAppLocaleSettingsdeclares@RequiresApi(TIRAMISU)instead of suppressingInlinedApi.6d29118FakeSharedPreferences reads string sets withfilterIsInstance, soUNCHECKED_CASTgoes away.7e38259The release DebugTools drops a staleUnusedParametersuppression.detektReleaseSourceSetpasses without it. The commit message wrongly says detekt does not analyze the release source set.113cfa7narrowed theopenURLcatch toSecurityException.e5157fbreverts that and keeps the broad catch with the error toast.Behavior changes
openURLkeeps its broad catch: every failure shows the error toast. Net behavior matchesmain.Coverage
Verification
spotlessCheck detekt lintDebug testDebugUnitTest koverVerifyDebug verifyRoborazziDebugpass at heade5157fb.assembleReleaseand GMDpixel9Api35DebugAndroidTestpass on an earlier commit of this branch.Notes
Kept suppressions, each with a reason:
RestrictedApi/PrivateResource.AppCompatResource/AlwaysShowAction.unusedandsem*SameReturnValue: Samsung calls these by reflection.DEPRECATION.QueryAllPackages(app picker) andExportedReceiver(Samsung facewidget).UnusedResources.unused.IncorrectFormatting.openURLTooGenericExceptionCaught: the broad catch stays by design.ButtonCase/Typos: the text is intended.UsePropertyAccessSyntax: IDE-only,setIs24HourView/is24HourViewform no Kotlin property.🤖 Generated with Claude Code
https://claude.ai/code/session_016XZN5ZKzGWBCHZwRwfNFtb
Summary
QSTileServiceand a test for the logged messages.@RequiresApi(TIRAMISU).FakeSharedPreferences.getStringSetand a stale release suppression.