ADFA-2320: Save progress when settings change#735
Conversation
|
To enable Rovo Dev code reviews, link your GitHub account to your Atlassian account. This is a one-time task that takes less than a minute. Once your account is linked, resubmit the pull request to trigger a code review. |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 Walkthrough
Risks & Best-practice concerns
Recommendations
WalkthroughThe activity's async save logic was made lifecycle-aware: imports for lifecycle APIs were added, Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt (1)
app/src/main/java/com/itsaky/androidide/interfaces/IEditorHandler.kt (2)
saveAllAsync(65-71)saveAll(52-57)
🔇 Additional comments (1)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt (1)
29-31: LGTM: Lifecycle imports added.The lifecycle-related imports are correctly added for the intended coroutine scoping changes.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt (1)
29-29: Remove unused lifecycle imports.The
LifecycleandrepeatOnLifecycleimports are no longer used in the code after the refactoring that removed therepeatOnLifecyclewrapper fromsaveAllAsync.Apply this diff to remove the unused imports:
-import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycleAlso applies to: 31-31
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt (1)
app/src/main/java/com/itsaky/androidide/interfaces/IEditorHandler.kt (2)
saveAllAsync(65-71)saveAll(52-57)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build Universal APK
🔇 Additional comments (1)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt (1)
518-522: LGTM! The lifecycle issue has been resolved.The refactored implementation correctly removes the
repeatOnLifecyclewrapper that was preventing execution fromonPause(). The simplelifecycleScope.launchapproach will:
- Execute immediately when called (including from
onPause())- Invoke the
runAftercallback exactly once after save completes- Allow the save operation to complete as long as the activity lifecycle allows
This properly addresses the critical issues identified in previous reviews.
* fix(ADFA-2320): Save modified files when leaving the app * fix(ADFA-2320): Use the appropriate coroutine scope
Save modified files when user is leaving the app so that progress is not lost.
Jira ticket
Screen_recording_20251219_180652.mp4