feat(windows): add support for winsparkle updates
#911
Conversation
aanorbel
force-pushed
the
feat/win-sparkle-updates
branch
from
434dcb1 to
be280d9
Compare
aanorbel
force-pushed
the
feat/win-sparkle-updates
branch
from
be280d9 to
f9f9754
Compare
aanorbel
force-pushed
the
feat/win-sparkle-updates
branch
from
977d281 to
1e00199
Compare
…obe-multiplatform into feat/win-sparkle-updates
|
|
||
| // Set shutdown callback for update installation (Windows and macOS) | ||
| when (updateManager) { | ||
| is org.ooni.probe.shared.WinSparkleUpdateManager -> { |
There was a problem hiding this comment.
Why have the package here, can't we import it?
| Item( | ||
| text = getUpdateMenuText(updateSystemState, updateSystemError), | ||
| enabled = updateSystemState != UpdateState.CHECKING_FOR_UPDATES, | ||
| onClick = { |
There was a problem hiding this comment.
This item clicks could be separate methods, to avoid growing the main method too much.
| Separator() | ||
| Item( | ||
| stringResource(Res.string.Desktop_Quit), | ||
| onClick = { |
There was a problem hiding this comment.
This one too could be a method.
| /** | ||
| * Sets up the update manager with basic error handling | ||
| */ | ||
| private suspend fun setupUpdateManager( |
There was a problem hiding this comment.
And this update methods could probably also go into a separate file.
| versionCode = 200 // Always increment by 10. See fdroid flavor below | ||
| versionName = "5.0.0" |
There was a problem hiding this comment.
We probably don't want to merge this, just use if for tests.
composeApp/build.gradle.kts
Outdated
| copyright = "© ${LocalDate.now().year} OONI. All rights reserved." | ||
| vendor = "Open Observatory of Network Interference (OONI)" | ||
| // licenseFile.set(project.file("LICENSE.txt")) | ||
| // licenseFile = rootProject.file("LICENSE") |
| fun getMenuText(): String = | ||
| when { | ||
| _error.value != null -> "Retry Update Check (Error: ${_error.value?.code})" | ||
| _state.value == UpdateState.CHECKING_FOR_UPDATES -> "Checking for Updates..." | ||
| _state.value == UpdateState.UPDATE_AVAILABLE -> "Update Available!" | ||
| _state.value == UpdateState.NO_UPDATE_AVAILABLE -> "Check for Updates (Up to date)" | ||
| _state.value == UpdateState.INITIALIZING -> "Initializing Updates..." | ||
| _state.value == UpdateState.ERROR -> "Update System Error - Retry" | ||
| else -> "Check for Updates..." | ||
| } |
There was a problem hiding this comment.
We need to localize all these messages, either here, or in the UI code. Could be in a separate ticket.
| } catch (e: Exception) { | ||
| Logger.e("Error during update manager cleanup: $e") | ||
| throw e |
There was a problem hiding this comment.
Why are we handling it here, if we're re-throwing the exception to be caught the same way by the caller? Either not handle it, and add a @Throws annotation, or handle it here and not re-throw it.
| onClick = { showWindow() }, | ||
| ) | ||
| // Only show update UI on Windows platforms | ||
| if ((dependencies.platformInfo.platform as? Platform.Desktop)?.os in listOf(DesktopOS.Windows)) { |
There was a problem hiding this comment.
This could be a method inside the UpdateController, to avoid having this logic here.
| // Show retry option when there are errors | ||
| if (updateError != null) { | ||
| Item( | ||
| "Retry Update Check", |
There was a problem hiding this comment.
Another string to localize. And I thought this came also from getMenuText().
No description provided.