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

fix: update to use SPUStandardUpdaterController from sparkle v2 #24

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

lanwen
Copy link
Contributor

@lanwen lanwen commented Dec 11, 2023

What this PR is doing

Fixes #11 by adopting the recommended SPUStandardUpdaterController which creates the default UI and necessary delegates.

Feed URL and decryption password settings migrated to SPUUpdaterDelegate as methods feedURLStringForUpdater and decryptionPasswordForUpdater, so to eliminate warnings, I had to implement those.

feedURLStringForUpdater calls go function to provide results, which could add some extensibility, however, I haven't changed the library API just yet to expose this way of setting the feed URL.

This PR should be backwards-compatible, but I was forced to export CGOFeedURL to use it in the ObjC code. Any suggestions on the naming will be appreciated to indicate, that this is an internal function and implementation detail.

I also slightly tampered with the example to make sure the affected methods did not cause any crashes and still worked fine.

I also had to add a clearFeedURLFromUserDefaults call, during my tests with the previous setFeedURL version it persisted the URL and didn't get back to the one from Info.plist. With this call that was fixed. So I suspect it's a necessary migration step

Why this PR is important

Having those deprecation warnings is annoying, so I thought It would be nice to finally migrate to a modern second version of the Sparkle API and get familiar with the binding in case I would need more methods exposed in the future

@lanwen lanwen changed the title SPUStandardUpdaterController from sparkle v2 fix: update to use SPUStandardUpdaterController from sparkle v2 Dec 11, 2023
@abemedia
Copy link
Owner

Amazing, thanks! I was a little too busy to review this but will do so in the next couple of days.

@lanwen
Copy link
Contributor Author

lanwen commented Dec 11, 2023

@abemedia no worries, it can wait, thank you

@lanwen
Copy link
Contributor Author

lanwen commented Feb 13, 2024

@abemedia Would you be able to find some time to take a look at the PR? We've been using the change in prod for a while and haven't noticed issues so far, so I can say it's safe enough, however, I would be happy to get any feedback on the code structure. I might be able to bring other APIs from the sparkle, but wouldn't want to go deep without the baseline aligned.

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.

Migrate from deprecated SUUpdater to SPUUpdater
2 participants