Skip to content

Merge onCleared into onStop#229

Merged
Goooler merged 1 commit into
trunkfrom
merge-on-cleard-to-on-stop
May 12, 2026
Merged

Merge onCleared into onStop#229
Goooler merged 1 commit into
trunkfrom
merge-on-cleard-to-on-stop

Conversation

@Goooler
Copy link
Copy Markdown
Owner

@Goooler Goooler commented May 12, 2026

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies several UI ViewModels by removing onCleared() overrides that only cancelled viewModelScope-launched Jobs, consolidating cleanup behavior around the existing DefaultLifecycleObserver.onStop() handling (and the ViewModel’s built-in viewModelScope cancellation on clear).

Changes:

  • Removed redundant onCleared() implementations from multiple ViewModels.
  • Keeps job cancellation logic in onStop() for lifecycle-driven cleanup.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
ui/proxy/src/main/kotlin/com/github/kr328/clash/proxy/vm/ProxyViewModel.kt Removes onCleared() cleanup; relies on existing onStop() cancellation and ViewModel clearing behavior.
ui/profile/src/main/kotlin/com/github/kr328/clash/profile/vm/ProvidersViewModel.kt Removes onCleared() that cancelled Jobs already bound to viewModelScope.
ui/profile/src/main/kotlin/com/github/kr328/clash/profile/vm/ProfilesViewModel.kt Removes redundant onCleared() job cancellation.
ui/home/src/main/kotlin/com/github/kr328/clash/home/vm/HomeViewModel.kt Removes onCleared() cleanup; lifecycle onStop() remains the active stop point.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Goooler Goooler merged commit 00e5d9e into trunk May 12, 2026
8 checks passed
@Goooler Goooler deleted the merge-on-cleard-to-on-stop branch May 12, 2026 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants