-
-
Notifications
You must be signed in to change notification settings - Fork 651
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
android: Drop support for API < 23. #4938
Conversation
<!-- When updating this, please update `docs/developer-guide.md` as well. --> | ||
|
||
[dropped-android-j]: https://chat.zulip.org/#narrow/stream/48-mobile/topic/platform.20versions/near/625585 | ||
[dropped iOS 8 support]: https://chat.zulip.org/#narrow/stream/48-mobile/topic/platform.20versions/near/628412 | ||
[dropped iOS 9 support]: https://chat.zulip.org/#narrow/stream/48-mobile/topic/platform.20versions/near/771761 | ||
[dropped-android-k]: https://chat.zulip.org/#narrow/stream/48-mobile/topic/platform.20versions/near/794551 | ||
[dropped iOS 11 support]: https://chat.zulip.org/#narrow/stream/48-mobile/topic/platform.20versions/near/1165087 | ||
[dropped-android-l]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Updating.20.60minSdkVersion.60/near/1236327 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice if someone could move conversations of that topic, to platform versions, to make this consistent with others.
(I think only admins can move conversations)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, maybe. I'm not really bothered by the inconsistency. @gnprice, are you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind the links not all being to the same topic. In fact it's convenient for the "platform versions" topic to not have much detailed discussion on it, to help make it easy to scan through it for the history of the numbers we've posted there, so I'm happy for the recent discussion to stay on a distinct topic.
It would be good to have a quick message there just recording that we've made this change, linking to the discussion thread and to this PR.
@gnprice, @chrisbobbe let me know if I missed anything, thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! See one small comment below. Also, I think there's one paragraph that can be deleted from a comment in src/webview/js/js.js 🎉:
* * Below Chrome 44, it's possible (but rare) for a user to be on a
* version as old as Chrome 37, which shipped with Android 5 Lollipop.
* We sometimes fix issues affecting those versions, only when trivial.
Hmm, and that makes me think we should maybe bump the numbers in the following, a bit above that:
* * For Android, core functionality needs to work on Chrome 44.
* Graceful degradation is acceptable below Chrome 74.
From a comment in docs/architecture/platform-versions.md, I think that the pattern would be to bump those two numbers to 51 and 83:
* Android versions 5 L, 6 M, 7 N, 8 O had originally shipped
with Chrome versions 37, 44, 51, 58 respectively (based on
looking at stock emulator images.)
* Later data: Android 9, 10, 11 ship with Chrome versions
69, 74, 83 respectively.
Maybe @gnprice could confirm? 🙂
docs/developer-guide.md
Outdated
@@ -1,6 +1,6 @@ | |||
# Developer guide | |||
|
|||
We target operating systems >= Android 5 Lollipop (API 21) | |||
We target operating systems >= Android 6 Marshmello (API 23) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "Marshmallow"
d930ec1
to
8c8b667
Compare
@chrisbobbe did the changes you asked for waiting on @gnprice to comment on
|
Yeah, there's probably a helpful related change to do here along those lines. It doesn't actually have a particular need to be coupled to this minSdkVersion change, though -- this one is about the minimum version of Android that we support, so the data it's based on is the distribution of Android versions our users have, while that one would be about the minimum versions of Chrome that we support to various degrees, so the data it'd be based on is the distribution of Chrome versions our users have. Those distributions are quite different from each other, because Android's Chrome auto-updates independently of the OS. Before this change, we nominally had three levels of support for different Chrome versions:
I'm kind of inclined to drop that lowest tier, because I'm not sure the extra distinction was really buying us that much. If we run into something that's busted only on ancient Chrome versions like 44, we can consider reintroducing it, or perhaps just dropping support for Android 6 M altogether (and consequently for Chrome older than 51.) The current version of this PR does that. Keeping it that way means leaving the mention of Chrome 44 as is, to be in sync with Android 6 M as our oldest supported Android version. For updating the "fully supported" / "graceful degradation below" version, it looks like our last update was 92d2402, earlier this year. Updating it to Chrome 83 would mean updating to a Chrome version released 2020-05 (thank you Wikipedia -- one of the niches Wikipedia tends to do a great job at is chronicling old versions of all kinds of major software). That seems pretty reasonable; that's over a year ago, 9 versions before the current latest, and our previous look at some data suggests that 90% of users will be on just the two latest versions. |
This all looks good; merging. Thanks @AkashDhiman for taking care of this! In the previous comment, I guess I reached the conclusion that a good followup would be to replace that mention of Chrome 74 with Chrome 83. I don't think that involves changing any other lines. A separate nice followup would be to remove the handful of spots in the For cutting those all out, it'd be good to try the app with Chrome 44 (by firing up an emulator with a stock Android 6 M image) and confirm that things seem to work. Also ideally on iOS 12, I guess (for the oldest Safari we support); but iOS 12 is a few years newer than Chrome 44, so even if our notes are mistaken it's a lot less likely that these features would be missing there. |
As Discussed in CZO [1], Last time (9-2020) we checked, the total user base using API < 23 is less than 1 percent and hence it is safe for us to update the API version (according to the update policy [2] we have). The advantage of this change would be simplification in some of the code that we use or will be using, which will be changed or added in other commits. The changes in this commit Involved: * Changing `android/build.gradle`. * Setting up chrome version to 44 (set according to what is written in `platform-versions.md`). * Modifications to the documentation. [1] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Updating.20.60minSdkVersion.60/near/1236327 [2] https://github.com/zulip/zulip-mobile/blob/master/docs/architecture/platform-versions.md#android-and-ios-versions
And merged! I made just one change, expanding the summary line: I tend to find the API numbers hard to keep track of, so when they appear in text I like to accompany them with the Android version number and letter (or the dessert the letter stands for, though starting with Android 9 P they've de-emphasized the letter and dessert, and starting with Android 10 Q the letter doesn't stand for anything.) Those have consistently advanced by one each year since before the oldest Android versions we care about, so I can always count backward from the latest to orient myself to how old a given number/letter is. |
As Discussed in CZO, Last time (9-2020) we checked, the total
user base using API < 23 is less than 1 percent and hence it is
safe for us to update the API version (according to the update
policy [2] we have).
The advantage of this change would be simplification in some of the
code that we use / will be using which will be changed / added in
other commits.
The changes in this commit Involved:
android/build.gradle
.written in
platform-versions.md
).Marking P1 since Notification UI is built on top of this PR.