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

Inflexible API #857

Closed
danielchasehooper opened this issue Aug 3, 2016 · 3 comments
Closed

Inflexible API #857

danielchasehooper opened this issue Aug 3, 2016 · 3 comments

Comments

@danielchasehooper
Copy link

Sparkle is great if all you want is the vanilla update process, but even slight changes to it's behavior are difficult to impossible.

While trying to use sparkle to add downgrade to support to Principleformac.com I made some notes:

  • All or nothing API. For the customization that is there, you either have to use it as is, or completely roll your own, even the parts that you aren't changing. Most of the bullet points below are incarnations of this problem.
    For example: Implementing bestValidUpdateInAppcast:forUpdater: requires you reimplement the max/min supported OS logic in addition to your novel verification logic. It would be nice if this logic were either already applied to the items that get sent to this delegate method and/or if SUAppcastItems had a -supportsOSVersion method
  • Doesn't allow extension without building from source
    If someone wanted to build their own update driver, they'd most likely have to end up building the framework from source, or do some nasty runtime hacking. It'd be nice if more headers were visible, even if just in a "Optional Headers" folder in the framework so that it's there for the people that need it. In my case, I'm wanting to use the basicUpdateDriver since it doesn't show any of it's own UI, but since the framework doesn't expose that class, so I'm getting the class by name at runtime - I don't want to add all Sparkle source to our project/build time.
  • Almost all SUUpdater delegate methods give you no control and act only as notifications. There have been lots of instances where the driver did mostly what I wanted, but I wanted to choose to show my own UI, show the alerts as a sheet (since they don't work when there is already a modal window), prevent the UI from showing all together. Example: -updater: shouldShowAlert: OfType:, which would allow you to control over how/if alerts are shown. It'd be nice if the delegate was delegated-to more.
  • huge number of delegate callback paths
    If you have logic that you need to guarantee runs after the update process has run you have to account for all the edge cases: user cancels, valid update couldn't be found, couldn't download update, etc. Looking at the source, I'm not even sure all paths are accounted for even if you implement all delegate methods. Some sort of will/didRunUpdateProcess might be helpful.
  • orthogonal concepts combined
    SUUpdateDriver combines all the logic relating to how the update is checked, installed and relaunched and it is also in charge of the user-facing aspect. There isn't a way to have one without the other. It may be worth considering splitting the two concepts into a SUUpdateDriver & SUUpdateUIProvider, for example - composition-based drivers would do this better than a class hierarchy.
@zorgiepoo
Copy link
Member

zorgiepoo commented Aug 3, 2016

I'm still working on a fork to resolve #363, but in the process I also decoupled all the UI and updater logic. If you're interested, take a look at the existing user driver classes. My intent is to make it so that people won't want to build their own update drivers, and it does away with all the subclassing hierarchy internally.

For the best valid appcast item, we at least export SUStandardVersionComparator which is better than nothing. Maybe we could supply a simpler support OS method if necessary, although I'm not sure it should belong in SUAppcastItem.

@danielchasehooper
Copy link
Author

danielchasehooper commented Aug 30, 2016

@zorgiepoo Any idea when your fork might make it into master?

@zorgiepoo
Copy link
Member

zorgiepoo commented Mar 26, 2021

Well, this is a really late reply but the 2.x branch switched to main development recently and the plan is to make a pre-release after polishing / reviewal. #1523 for migration status.

Most of these concerns have been addressed directly in 2.x, and a handful of apps have already been using this in production for a few years now due to sandbox support.

Nova by Panic for example uses Sparkle 2 to good use and implements their own UI:

Screen Shot 2021-03-26 at 12 29 09 AM

bestValidUpdateInAppcast:forUpdater: requiring you reimplement the max/min supported OS was basically a bug I addressed in 2.x recently.

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

No branches or pull requests

2 participants