Conversation
* prevented new installs from showing changelog * Added filters for all brackets in lyrics search
* Added github sponsors link to foss paywall and about page and changelog sheet
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe release updates version metadata and changelogs, normalizes song titles and artists, refactors lyrics search and fetching around cancellable jobs, adds conditional changelog support navigation, and adds GitHub Sponsors UI actions. ChangesLyrics normalization and search
Changelog and support UI
Release metadata and toolchain
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MediaListener
participant SearchSheetVM
participant Util
participant LyricsRepository
MediaListener->>SearchSheetVM: Emit song metadata
SearchSheetVM->>Util: Normalize title and artist
Util-->>SearchSheetVM: Return main title and artist
SearchSheetVM->>LyricsRepository: Search or fetch lyrics
LyricsRepository-->>SearchSheetVM: Return results or lyrics
SearchSheetVM-->>SearchSheetVM: Update search and lyrics state
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
shared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/component/ChangelogSheet.kt (1)
147-147: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract hardcoded strings to resource files for localization.
These newly added UI elements use hardcoded English text, which makes them inconsistent with adjacent elements (like the BMC button) that correctly utilize
stringResourcefor internationalization.
shared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/component/ChangelogSheet.kt#L147-L147: extract"Support Rush"to a string resource.shared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/component/FossPaywall.kt#L139-L139: extract"GitHub Sponsors"to a string resource.shared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/setting/section/About.kt#L188-L189: extract"GitHub Sponsors"and"Support me through GitHub"to string resources.shared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/lyrics/component/PlainLyrics.kt#L272-L272: extract"Genius Source"to a string resource.🤖 Prompt for 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. In `@shared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/component/ChangelogSheet.kt` at line 147, Replace the hardcoded UI labels with localized string resources, following the existing stringResource pattern: update ChangelogSheet.kt lines 147-147 for “Support Rush”, FossPaywall.kt lines 139-139 for “GitHub Sponsors”, About.kt lines 188-189 for both “GitHub Sponsors” and “Support me through GitHub”, and PlainLyrics.kt lines 272-272 for “Genius Source”; add the corresponding resource entries and use them in each component.shared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/viewmodels/SearchSheetVM.kt (1)
89-108: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant coroutine nesting in
observeSongInfo.
viewModelScope.launch { flow.onEach{...}.launchIn(this) }wraps a collector launch inside another launch with no added value. Collecting directly (onEach{...}.launchIn(viewModelScope)) and assigning thatJobtoobserveSongInfoJobis equivalent and simpler to reason about for cancellation.🤖 Prompt for 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. In `@shared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/viewmodels/SearchSheetVM.kt` around lines 89 - 108, Update observeSongInfo to remove the outer viewModelScope.launch around the MediaListener.songInfoFlow collection. Keep the existing distinctUntilChanged and onEach logic, launch the flow directly in viewModelScope, and assign the returned Job to observeSongInfoJob so cancellation remains unchanged.
🤖 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
`@shared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/viewmodels/GlobalVM.kt`:
- Around line 104-107: Update the changelog handling in the launch coroutine
around lastShownChangelog and changelogManager.changelogs so that when
lastShownChangelog is blank, it fetches the latest changelog version, persists
that version, and then returns without displaying the changelog. Preserve the
existing display behavior for subsequent launches.
In
`@shared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/viewmodels/SearchSheetVM.kt`:
- Around line 163-185: The search flow in the coroutine around repo.searchGenius
should update _lastSearched only when the result is Result.Success. Remove the
unconditional _lastSearched update from finally while preserving the existing
cleanup there, so failed searches can be retried with the same query.
- Around line 216-226: The fetchLyrics method currently cancels the previous job
and then returns when LyricsState is Fetching, dropping replacement requests.
Move the Fetching guard before fetchJob?.cancel() if only one request may run,
or remove that guard to allow the newly requested song to start; update the
logic in fetchLyrics accordingly.
---
Nitpick comments:
In
`@shared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/component/ChangelogSheet.kt`:
- Line 147: Replace the hardcoded UI labels with localized string resources,
following the existing stringResource pattern: update ChangelogSheet.kt lines
147-147 for “Support Rush”, FossPaywall.kt lines 139-139 for “GitHub Sponsors”,
About.kt lines 188-189 for both “GitHub Sponsors” and “Support me through
GitHub”, and PlainLyrics.kt lines 272-272 for “Genius Source”; add the
corresponding resource entries and use them in each component.
In
`@shared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/viewmodels/SearchSheetVM.kt`:
- Around line 89-108: Update observeSongInfo to remove the outer
viewModelScope.launch around the MediaListener.songInfoFlow collection. Keep the
existing distinctUntilChanged and onEach logic, launch the flow directly in
viewModelScope, and assign the returned Job to observeSongInfoJob so
cancellation remains 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
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 572b058b-f44d-45e6-ac6a-156654586a03
📒 Files selected for processing (15)
CHANGELOG.mdandroidApp/build.gradle.ktsandroidApp/src/main/java/com/shub39/rush/app/App.ktdesktopApp/src/commonMain/kotlin/com/shub39/rush/app/App.ktgradle/libs.versions.tomlshared/core/src/commonMain/kotlin/com/shub39/rush/shared/core/Util.ktshared/core/src/commonTest/kotlin/TTMLParserTest.ktshared/core/src/commonTest/kotlin/UtilTest.ktshared/logic/src/commonMain/composeResources/files/changelog.jsonshared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/component/ChangelogSheet.ktshared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/component/FossPaywall.ktshared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/lyrics/component/PlainLyrics.ktshared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/setting/section/About.ktshared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/viewmodels/GlobalVM.ktshared/ui/src/commonMain/kotlin/com/shub39/rush/shared/ui/viewmodels/SearchSheetVM.kt
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.
Summary by CodeRabbit