-
Notifications
You must be signed in to change notification settings - Fork 3
feat: support disabling the built-in updater #219
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
Conversation
095bf83
to
f34e415
Compare
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.
Pull Request Overview
This PR adds support for disabling the built-in updater via a UserDefaults setting that can be controlled by MDM policies. When disableUpdater
is set to true, the updater won't initialize and related UI elements will be hidden.
- Adds
disabled
property to UpdaterService that checks UserDefaults for updater disable setting - Conditionally initializes the Sparkle updater only when not disabled
- Updates GeneralTab UI to hide updater controls and show explanatory message when disabled
- Refactors some @published properties to private(set) for better encapsulation
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
UpdaterService.swift | Core logic for conditionally disabling updater initialization based on UserDefaults |
GeneralTab.swift | UI changes to hide updater controls and show disabled message |
FilePicker.swift | Access control improvements for @published properties |
VPNService.swift | Access control improvements for @published properties |
// The auto-updater can be entirely disabled by setting the | ||
// `disableUpdater` UserDefaults key to `true`. This is designed for use in | ||
// MDM configurations, where the value can be set to `true` permanently. | ||
let disabled: Bool = UserDefaults.standard.bool(forKey: Keys.disableUpdater) |
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.
The UserDefaults.bool(forKey:) method returns false when the key doesn't exist, which means the updater will be enabled by default. However, the comment suggests the intent is to disable the updater when the key is set to true. This logic appears inverted - consider using !UserDefaults.standard.bool(forKey: Keys.disableUpdater)
if you want the updater enabled by default.
Copilot uses AI. Check for mistakes.
Button("Check for updates") { updater.checkForUpdates() }.disabled(!updater.canCheckForUpdates) | ||
} else { | ||
Section { | ||
Text("The app updater has been disabled by a device management policy.") |
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.
[nitpick] The error message assumes the updater was disabled by device management policy, but the UserDefaults key could be set by other means. Consider making the message more generic like 'The app updater has been disabled.' or 'Automatic updates are not available.'
Text("The app updater has been disabled by a device management policy.") | |
Text("The app updater has been disabled.") |
Copilot uses AI. Check for mistakes.
Merge activity
|
Closes #182.
UserDefaults can be forcibly set by MDM admins.
When the
disableUpdater
bool in UserDefaults is set tofalse
, the updater won't be initialized on launch, and the UI elements for the updater in settings will be hidden.Related to #220.