search

thunder-app / thunder

Thunder - An open-source cross-platform Lemmy client for iOS and Android built with Flutter
https://thunderapp.dev
GNU Affero General Public License v3.0
732 stars 62 forks source link

Fix version parsing #478

Closed micahmo closed 8 months ago

micahmo commented 10 months ago

For in-app update notifications, our current version parsing was a simple string comparison. This hasn't been working well lately, especially with our extended usage of semver. The version package helps with that.

Ref: https://github.com/thunder-app/thunder/issues/266#issuecomment-1650335600

Issue Number: #615

micahmo commented 10 months ago

It also occurred to me that we're only grabbing the latest full release, which is another reason @krestenlaust wasn't getting update notifications. Now that the app is on Google Play and TestFlight, maybe the in-app notifications should include alphas and be off by default. Thoughts on that?

krestenlaust commented 10 months ago

Agree, I think it would be good with an option to opt-out of alpha releases, but allow people who want alpha versions like me, to still be notified.

micahmo commented 10 months ago

Ok I updated the PR to disable in-app update notifications by default. I am going to make this a draft until we have a new prerelease alpha, then I can verify that we always get the "newest" as opposed to the "latest" (which are only the stable ones).

krestenlaust commented 10 months ago

It is my belief that the notifications for full releases should be on by default, and alpha-releases are opt-in

hjiangsu commented 10 months ago

We would also need to do testing on iOS devices for this

micahmo commented 10 months ago

TestFlight builds are only for the full releases and not nightlies/pre-releases

Yes, analogous to Google Play Store releases in that way. That's why I was suggesting the in-app notifications could be for prereleases, since we'll assume you'll get stable releases through other channels.

Versioning is different in iOS than it is on Android (iOS only allows numerical values and does not contain any characters)

Ahh, maybe that's why the string comparison worked for iOS. Maybe we can inject the current full version number somewhere else (even if it's a const String we stick somewhere) so we have a consistent thing to check.

micahmo commented 9 months ago

Hey all, I'm finally revisiting this issue. I've considered quite a lot of alternatives here, like multiple options for stable/prerelease, automatic in-app updates (including Android's native one), differentiating between different install sources, etc. For posterity, here are all the packages related to this topic that I found interesting.

However, I think in-app update notifications are mainly geared toward those who did not install through a store. Therefore, I think what we have in this PR is very close to what we want, with a couple more additions:

In the future, we could do something more fancy, like checking whether the app was installed via the store, and showing the native store updater.

In order to avoid the asynchronous nature of GitHub discussions, I'll move forward with my idea here, but please leave feedback!

micahmo commented 9 months ago

This will probably not go in the next GA, so I'll leave it as as draft, but just for future reference, this should be ready to go once #574 is done.

hjiangsu commented 9 months ago

With the pending release for Thunder on the Apple App Store, I was thinking maybe we start using the normal semver conventions for releases/nightlies

My initial thought on this is:

micahmo commented 9 months ago

I'm all in favor of following semver more closely! I think the problem with using the build number is that it's not included in the sermver comparison. I understand that iOS doesn't support alpha characters. Could we use the prelease moniker with just a number? 0.2.2-1, 0.2.2-2? I'd have to make sure sorting works, but that might satisfy all of the requirements.

hjiangsu commented 9 months ago

Hmm, that might be possible. I just hope that doesn't cause more confusion for users when describing the pre-release versions (having alpha helps distinguish that a version is a pre-release as compared to a general release whereas just having the value might make it less obvious)

micahmo commented 9 months ago

(having alpha helps distinguish that a version is a pre-release as compared to a general release whereas just having the value might make it less obvious)

I agree, but since iOS doesn't support alpha characters, I think we have to come up with some other solution, right? Otherwise it'll always be hard for us to grab the "real" version from iOS builds. Since you already suggested using just a single number for nightlies, I was thinking we could do that the "right" way according to semver.

Would it help if tapping the version number copied it to the clipboard? Then users might have an easier time sending us their version number.

hjiangsu commented 9 months ago

@micahmo I've added a few changes here!

Here are the screenshots:

For 0.2.4-1+17 (iOS and Android respectively): image image

For 0.2.4+17 (iOS and Android respectively):: image image

Feel free to test it out again on Android and let me know if it's working as intended! We can use this in addition to the CI check that you created previously to keep the versions in sync between pubspec.yaml and the globals file.

hjiangsu commented 9 months ago

One thing to note is that changes may be needed for the comparison logic to check for new updates. My plan for the release tag will be to include everything except the internal build number.

For example: