Skip to content
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

Add automatically-install-updates option #620

Merged
merged 1 commit into from Oct 8, 2015

Conversation

statico
Copy link
Contributor

@statico statico commented Sep 18, 2015

Sparkle is great. Since we're in beta and we want to keep our beta testers as up-to-date as possible, I wanted a flow that would check for updates and install them automatically when the application is launched. This change does that.

  SUUpdater* updater = [SUUpdater sharedUpdater];
  [updater setDelegate:self];
  [updater installUpdatesIfAvailable];

The above checks for updates interactively using the Sparkle progress windows. If an update is found, it is automatically downloaded, installed, and the application is restarted. The application can respond to updaterDidNotFindUpdate as usual, or do something when userDidCancelDownload (a new SUUpdaterDelegate method) is called.

cc @mlogan @BKcore

@statico
Copy link
Contributor Author

statico commented Sep 18, 2015

Also, I'm not sure if this is functionality that the Sparkle project wants. If it is, I'll extend the PR with tests if possible.

@jakepetroules
Copy link
Contributor

Also, I'm not sure if this is functionality that the Sparkle project wants.

Yeah, I think it's a valuable feature. Perhaps this might have been frowned upon in the past but this technique is used by many popular applications today and as long as it's user-configurable I think it's great.

@zorgiepoo
Copy link
Member

How is this better than the Automatic Updates feature Sparkle currently provides (see SUAutomaticUpdateDriver)? I actually think this is better because it installs an update on termination without the user noticing. I feel like I'm missing something here.

@statico
Copy link
Contributor Author

statico commented Sep 19, 2015

@jakepetroules Cool. I'll make some tests, then.

@zorgiepoo We're using Sparkle in a wrapper application whose purpose is solely to check for updates, respond to custom URI schemes, and launch the main application which is a giant cross-platform blob. I didn't think the SUAutomaticUpdateDriver lets us check for updates and install them while showing a progress/feedback window.

@zorgiepoo
Copy link
Member

Right, SUAutomaticUpdateDriver covers a case for those wanting to do automatic non-intrusive in-the-background updates..

So if I understand correctly, your changes would allow an app, after it checks for updates, to automatically download the update, show the user UI install progress & allow them to cancel the download, and once the update is downloaded & unarchived to automatically relaunch the application without user input?

This sounds like it's designed in a way such that updates should only be checked on launch, and that the updater's automaticallyChecksForUpdates should never be YES

Personally I'm not too much of a fan of this, and the scenario you give about having a wrapper app doesn't sound like a common case to me. Adding mostly-unused features can have hidden maintainability costs. Wonder what @pornel thinks.

@statico
Copy link
Contributor Author

statico commented Sep 19, 2015

@zorgiepoo Correct, that's the use case and we set setAutomaticallyChecksForUpdates to false.

@kornelski
Copy link
Member

I see it may be useful for frequently-updated betas/nightlies, but it has a potential to be very annoying.

The fact that you need to invoke 3 methods makes it seem like a hack. Perhaps the API should be something like:

[sparkle installUpdatesIfAvailable];

that does an immediate check + install, regardless of other settings.

@kornelski
Copy link
Member

@statico I think the functionality is useful, but a better API would be just a command that does check+install on demand. Would you be interested in changing the code to do that?

@statico
Copy link
Contributor Author

statico commented Sep 27, 2015

@pornel Sure, I could do that. It'll take me a few days

@statico statico force-pushed the artillery-seamless branch 2 times, most recently from 30657b4 to 87398bc Compare October 8, 2015 21:28
@statico
Copy link
Contributor Author

statico commented Oct 8, 2015

@pornel API updated to a single method as suggested, [updater installUpdatesIfAvailable]. Please take another look.

kornelski added a commit that referenced this pull request Oct 8, 2015
Add automatically-install-updates option
@kornelski kornelski merged commit ef61691 into sparkle-project:master Oct 8, 2015
@kornelski
Copy link
Member

Thank you

@zorgiepoo
Copy link
Member

Should the updater:didFindValidUpdate: delegate method be called? This patch is skipping it.

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.

None yet

4 participants