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

Skip minor updates separately from major upgrades #1853

Merged
merged 17 commits into from
May 30, 2021
Merged

Conversation

zorgiepoo
Copy link
Member

@zorgiepoo zorgiepoo commented May 10, 2021

Handle skipping minor updates separately from major upgrades (in the case of paid upgrades).

If the user skips a major upgrade, they skip future updates belonging to that upgrade (similar to if a developer decides to use a different appcast feed). But they also now do not skip updates to their current / older major versions, allowing developers to publish a fix to the last major version without it being automatically skipped.

Fixes #329 (along with #1850)

User driver update found API now encapsulates information in a state object (including its stage, if it is a user initiated, or if update is major one). I removed some legacy deprecated user drivers APIs that were becoming painful to support / will now result in compiler errors if not migrating to new APIs.

We also re-title install/skip buttons in the case of a major upgrade (i.e, "Skip Major Upgrade" instead of "Skip this Version", "Install Upgrade" instead of "Install Update")

Checklist:

  • My change is being tested and reviewed against the Sparkle 2.x branch. New changes must be developed on the 2.x development branch first.
  • My change is being backported to master branch (Sparkle 1.x). Please create a separate pull request for 1.x, should it be backported. Note 1.x is feature frozen and is only taking bug fixes, localization updates, and critical OS adoption enhancements.
  • I have reviewed and commented my code, particularly in hard-to-understand areas.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • My change is or requires a documentation or localization update - Will update the paid section on our doc websites at a later point.

Testing

I tested and verified my change by using one or multiple of these methods:

  • Sparkle Test App
  • Unit Tests
  • My own app
  • Other (please specify)

Added skipped update tests with minimumAutoUpdate to test correct behavior.
Tested altering sparkle test app appcast and inspecting user defaults when skipping one or multiple updates, and making sure updates are correctly skipped.

I tested all of these:

  • User is on 1.0. Updates are 1.5, and 2.0 (min autoupdate version to 1.5). User is presented 1.5, if you skip it, user will then automatically be presented 2.0.

  • User is on 1.0. Updates are 2.0 (min autoupdate version to 1.5). User is presented 2.0, if you skip it, user will not automatically be shown new updates. If a new item shows up in the feed as 1.4 however, 1.4 will be automatically presented to user and 2.0 will stay skipped.

  • User is on 1.0. Updates are 2.0 (min autoupdate version to 1.5). User is presented 2.0, if you skip it, user will not automatically be shown new updates. If a new item shows up in the feed as 2.1 (same min autoupdate version to 1.5), 2.1 will also be skipped automatically.

  • User is on 1.0. Updates are 1.5. User is presented 1.5, if you skip it, user will not automatically be shown new updates. If a new item shows up in the feed as 1.6, 1.6 will be automatically presented to user.

  • User is on 1.0. Updates are 1.4 and 1.5. User is presented 1.5, if you skip it, user will not automatically be shown new updates.

  • User initiating for updates themselves clears out all skipped updates.

  • Skipping a new update replaces currently stored skipped update for that same train / minimumAutoupdateVersion.

macOS version tested: 11.3 (20E232)

We unify the user driver method showing the update to take a generic state object now. The current fields of the state object are its current stage, if it's user initiated, and if the update is a major one.

We also remove deprecated code paths that were complex to support and will otherwise generate in compile errors if people adopt the new API.
@zorgiepoo zorgiepoo changed the title Skip minor updates differently from major upgrades Skip minor updates separately from major upgrades May 10, 2021
@kornelski
Copy link
Member

I'm not sure about exposing this in the UI. It adds new strings to translate, and I don't think users will notice subtle differences between Update and Upgrade.

@kornelski
Copy link
Member

kornelski commented May 10, 2021

Could this all be simplified to make "skip" not actually permanently skip anything, but only silence notifications until the feed changes in any way?

This change adds a lot of new logic to respect user's skip choice, but IMHO "skip" is a bad idea to begin with. Users shouldn't be skipping updates, only delay them a bit if they come at inconvenient time.

Some devs use Sparkle to sell major upgrades, but I don't think that's a good use-case for Sparkle. We're not a shop.

@zorgiepoo
Copy link
Member Author

zorgiepoo commented May 11, 2021

Could this all be simplified to make "skip" not actually permanently skip anything, but only silence notifications until the feed changes in any way?

Yes, we can choose to not let the user skip future updates to a major upgrade. It all depends on what we want and leads to the best user experience; there are some tradeoffs.

We should have a best supported or documented way of dealing with paid (or major, not necessarily paid, eg like how OS or complex / pro app ones may not be paid) upgrades, something more than "major updates are not a recommended use case with Sparkle"..

We've already acknowledged that developers should be able to stick a major upgrade in the same feed as the last major version and that we need logic to alert the user and prevent a major upgrade from being installed automatically.

So back to skipping updates, do we want:

  • To only support skipping a specific version of a major upgrade and not later versions of that upgrade.
    ** Users that have reason to stay on previous major version may become annoyed of new update alert windows each time a minor update that is not applicable to them is released, without any way of opting out in advance.
    ** User may give into disabling automatic update checking (if app provides a UI) which may prevent them for getting the occasional minor update / security release they would want to get. If they eventually want to install a major upgrade, they may forget to re-enable automatic update checking.
  • To support skipping updates to a major upgrade
    ** User may in some cases want to skip a specific version of the upgrade but want to be reminded later of a new version of the same upgrade, which they cannot do with this choice.
    ** If they skip an upgrade, this is somewhat similar to developers that spin off a new feed for an upgrade and skipping the update in the old feed.
  • To let the feed point to an old release or an informational one, and new major version pointing to different feed
    ** Old feed will now either become out of date or require maintenance of multiple feeds (more likely former)
    ** Updating is less seamless from older version in either case (updates to an older version requiring multiple steps, or directs user to website which may involve worse or less secure installation experience)
    ** I want to address common needs / supported features so they do not require spinning off a new feed (related: Add option to have multiple updates in a single appcast that can be selectively chosen #51, Critical and informational updates are not selectively chosen #1855)

Skipping in all cases only refers to automatic scheduled checks; users can initiate update checks themselves.

The way skipping currently operates in Sparkle is that users are given control of not wanting to install a specific update (otherwise they can dismiss it if it comes at an inconvenient time).

In any case, because we already support paid/major updates to some degree (by minimumAutoupdateVersion), we should not implicitly skip updates below a major version. The best / simplest future-proof way to do this and detect if the feed changed is probably still to record the set of versions / minimumAutoupdateVersions that were skipped, so whatever we go with I don't imagine the implementation differing that much.

@zorgiepoo
Copy link
Member Author

zorgiepoo commented May 11, 2021

I'm not sure about exposing this in the UI. It adds new strings to translate, and I don't think users will notice subtle differences between Update and Upgrade.

I think regardless of any skipping logic, having more clarity that an update is a major upgrade outside of the release notes is a decent addition. I do think you are right the subtle strings I added may not be that great way of doing this (although Skip Major Upgrade is more correct than Skip This Version if we go with my initial proposal). Once there is the right way to do this (I'm not that interested having to solve this right now), then translations can follow along.

@zorgiepoo
Copy link
Member Author

zorgiepoo commented May 15, 2021

I've been thinking about this in the background a bit about other possibilities, but ultimately I still believe:

  • Users should be able to download and install a major version from within the application using Sparkle
  • Users should be able to opt-out of minor update notifications to a major upgrade, without disabling the updater system

Major update: an update significant in changes that needs more notice from user; often paid, but otherwise may present compatibility changes, significant overhauls, or workflow changes.

Disabling auto-updates is in progression to this. It's not a shop. It's presenting the user a choice of installing an update and deciding/respecting if they want to install it. Other application-specific details need to be handled separately, much like in the case if the user installs the update manually.

Developers that don't like this approach don't have to use this but developers that disable auto-updates for a major upgrade need to allow users to be able to opt-out properly.

@zorgiepoo
Copy link
Member Author

zorgiepoo commented May 29, 2021

I made changes to greatly simplify the implementation here by tracking a skipped major version and a skipped minor version, so up to two versions can be skipped, instead of us tracking an array of version pairs.

Instead of subtly renaming install/skip buttons, I added a confirmation alert for users skipping future updates to an upgrade.

This fills in the gap of what developers are currently doing for major upgrades and doesn't leave us in a currently half-supported (just preventing autoupdates) state for them. There's also user driver API changes encapsulating the state and cleanup here I need in the future.

@zorgiepoo zorgiepoo merged commit d7c504a into 2.x May 30, 2021
@zorgiepoo zorgiepoo deleted the skip-major-updates branch May 30, 2021 06:19
1024jp added a commit to 1024jp/Sparkle that referenced this pull request May 30, 2021
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.

Set minimum application version for update
2 participants