-
Notifications
You must be signed in to change notification settings - Fork 75
Minor: Remove duplicated log lines #377
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
base: master
Are you sure you want to change the base?
Minor: Remove duplicated log lines #377
Conversation
Moves the Timber log listener instantiation to the top of the class and removes it on service destruction for proper logging behavior. Adds a prefix "Main:" or "UserLogin:" to Timber log messages to quickly identify the source of events.
📝 WalkthroughWalkthroughPromoted a local logger to a class-level LogListener in SteamService with explicit add/remove lifecycle calls; standardized Timber logging to use explicit tags (e.g., "MainViewModel", "UserLoginViewModel") across two ViewModel classes and added one Steam event unsubscription in MainViewModel.onCleared. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
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 |
Adds a Timber tag to logs within MainViewModel and UserLoginViewModel. This improves log filtering and debugging by providing context.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/ui/model/UserLoginViewModel.kt (1)
223-236: Critical: Missing event unsubscription causes memory leak.The
SteamEvent.RemotelyDisconnectedevent is subscribed at line 207 but never unsubscribed inonCleared. This prevents theonRemoteDisconnectedhandler from being garbage collected, causing a memory leak.🔎 Proposed fix
override fun onCleared() { Timber.tag("UserLoginViewModel").d("onCleared") PluviaApp.events.off<SteamEvent.Connected, Unit>(onSteamConnected) PluviaApp.events.off<SteamEvent.Disconnected, Unit>(onSteamDisconnected) PluviaApp.events.off<SteamEvent.LogonStarted, Unit>(onLogonStarted) PluviaApp.events.off<SteamEvent.LogonEnded, Unit>(onLogonEnded) PluviaApp.events.off<AndroidEvent.BackPressed, Unit>(onBackPressed) PluviaApp.events.off<SteamEvent.QrChallengeReceived, Unit>(onQrChallengeReceived) PluviaApp.events.off<SteamEvent.QrAuthEnded, Unit>(onQrAuthEnded) PluviaApp.events.off<SteamEvent.LoggedOut, Unit>(onLoggedOut) + PluviaApp.events.off<SteamEvent.RemotelyDisconnected, Unit>(onRemoteDisconnected) SteamService.stopLoginWithQr() }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/app/gamenative/ui/model/MainViewModel.ktapp/src/main/java/app/gamenative/ui/model/UserLoginViewModel.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
app/src/main/java/app/gamenative/ui/model/UserLoginViewModel.kt (1)
37-37: LGTM! Consistent Timber tagging improves log traceability.The explicit Timber tags consistently identify the source ViewModel across all logging statements, making debugging and log filtering straightforward.
Also applies to: 52-52, 76-76, 99-99, 110-110, 117-117, 128-128, 184-184, 197-197, 211-211, 224-224, 279-279, 328-328
app/src/main/java/app/gamenative/ui/model/MainViewModel.kt (2)
79-79: LGTM! Consistent Timber tagging enhances observability.All logging statements now use the explicit
"MainViewModel"tag, making it easy to filter and trace events originating from this ViewModel.Also applies to: 84-84, 89-89, 99-99, 106-106, 113-115, 367-367, 383-383
149-158: Excellent fix! Added missing event unsubscription.The
SteamEvent.LogonStartedevent subscription (line 132) now has a corresponding unsubscription inonCleared, preventing a potential memory leak. This directly addresses the PR objective.
Moves the Timber log listener instantiation to the top of the class and removes it on service destruction for proper logging behavior.
Added missing off event at MainViewModel
Adds a prefix "Main:" or "UserLogin:" to Timber log messages to quickly identify the source of events.
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.