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

ios: Drop iOS 11 support; require iOS 12.1+. #4664

Merged
merged 2 commits into from
Apr 20, 2021

Conversation

chrisbobbe
Copy link
Contributor

@chrisbobbe chrisbobbe commented Apr 15, 2021

ios: Drop iOS 11 support; require iOS 12.1+.

We'd like to use Array.prototype.flatMap, which it seems won't
work in React Native on iOS until iOS 12. This was noted in a
comment [1] on facebook/react-native#26283. While the reasoning
there wasn't as thorough as we'd like, we looked closer [2], and we
believe the conclusion is correct.

A fresh look at iOS installations by platform (or the proxy we use
for that, anyway) [3] shows iOS 11 usage is 0.1% of our iOS usage,
so this should be fine to do. Also, 5.9% of our iOS users are on iOS
<= 13, so graceful degradation at that level is fine.

[1] facebook/react-native#26283 (comment)
[2] https://chat.zulip.org/#narrow/stream/48-mobile/topic/platform.20versions/near/1163034
[3] https://chat.zulip.org/#narrow/stream/48-mobile/topic/platform.20versions/near/1164857

@chrisbobbe chrisbobbe requested a review from gnprice April 15, 2021 15:02
@chrisbobbe chrisbobbe changed the title [DRAFT] ios: Drop iOS 11 support; require iOS 12.1+. ios: Drop iOS 11 support; require iOS 12.1+. Apr 16, 2021
@chrisbobbe chrisbobbe marked this pull request as ready for review April 16, 2021 18:39
@chrisbobbe
Copy link
Contributor Author

(For comparison, c953bc3 was the commit in which we dropped iOS 10 support.)

@gnprice
Copy link
Member

gnprice commented Apr 16, 2021

Thanks @chrisbobbe ! Small comments:

  • Let's also update tools/generate-webview-js; see 84fae48. I think it won't actually change the output (because our minimum Chrome version is rather older), but good to keep in sync.
  • In the "history" entry in platform-versions.md, I've liked having links to the chat threads where we made the decision; sometimes handy for looking those up, and easy to do at the time we're making the change.
  • In the commit message: I don't feel like our reasoning on the "RN uses the system JSC" point is totally thorough either, so I'd just drop that bit.

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 16, 2021
This will let us make `prevMessage` non-stateful, which we'll do in
the next commit, and probably make way for further tidying of this
code.

Requires dropping iOS 11 support; see zulip#4664.
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 16, 2021
This will let us make `prevMessage` non-stateful, which we'll do in
the next commit, and probably make way for further tidying of this
code.

Requires dropping iOS 11 support; see zulip#4664.
@chrisbobbe
Copy link
Contributor Author

chrisbobbe commented Apr 16, 2021

  • Let's also update tools/generate-webview-js; see 84fae48. I think it won't actually change the output (because our minimum Chrome version is rather older), but good to keep in sync.

Goodness, right! Thanks for the reminder; I see you carefully made sure there were bidirectional comments, and I looked right past them, only focusing on what I'd done in my own commit last time. 🙂

New revision pushed.

Copy link
Contributor

@WesleyAC WesleyAC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two minor comments, but looks good other than those — feel free to merge once you've fixed them.

@@ -91,11 +91,15 @@ History:
* We dropped iOS 10 support in 2020-10. It was 0.3% of iOS users who
tried Zulip, and we wanted to use a feature introduced in iOS 11,
called "named colors".
* We [dropped iOS 11 support][] in 2021-04. It was 0.1% of iOS users
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the empty [] doesn't do anything here, I don't think

@@ -91,11 +91,15 @@ History:
* We dropped iOS 10 support in 2020-10. It was 0.3% of iOS users who
tried Zulip, and we wanted to use a feature introduced in iOS 11,
called "named colors".
* We [dropped iOS 11 support][] in 2021-04. It was 0.1% of iOS users
who tried Zulip. We started iOS 12 support at 12.1 because Xcode's
dropdown for "Deployment Target" didn't have a 12.0.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment here like:

<!-- When updating this, please update `docs/developer-guide.md` as well. -->

Alternatively, we might want to consider just having fewer places where we keep this information :) I'd be fine with removing it from docs/developer-guide.md altogether, or keeping a link to this doc but not explicitly writing down the versions.

A fresh look at iOS installations by platform (or the proxy we use
for that, anyway) [1] shows iOS 11 usage is 0.1% of our iOS usage,
so we're well into the time when this is OK to do.

Also, with 1.5% of users on 12 and older, iOS 12 is solidly in the
range where we'd be happy to drop it, if we find things don't work
or require effort to keep working there.

5.9% of our iOS users are on iOS <= 13, so graceful degradation at
that level is fine.

[1] https://chat.zulip.org/#narrow/stream/48-mobile/topic/platform.20versions/near/1164857
@chrisbobbe chrisbobbe merged commit 7336fb1 into zulip:master Apr 20, 2021
@chrisbobbe
Copy link
Contributor Author

Thanks for the review! Merged, with those tweaks.

@gnprice
Copy link
Member

gnprice commented Apr 20, 2021

Thanks!

  • In the commit message: I don't feel like our reasoning on the "RN uses the system JSC" point is totally thorough either, so I'd just drop that bit.

FTR, what I meant here (and, rereading, I see this was thoroughly unclear in my comment) was that the reasoning that we want to use flatMap and we believe that's only available on iOS 12+ is fine -- just that I'd like to drop the part about how the reasoning in that other comment wasn't thorough.

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 21, 2021
This will let us make `prevMessage` non-stateful, which we'll do in
the next commit, and probably make way for further tidying of this
code.

Requires dropping iOS 11 support; see zulip#4664.
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Jun 8, 2021
This will let us make `prevMessage` non-stateful, which we'll do in
the next commit, and probably make way for further tidying of this
code.

Possible now that we've dropped iOS 11 support; that was zulip#4664.
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Jun 8, 2021
This will let us make `prevMessage` non-stateful, which we'll do in
the next commit, and probably make way for further tidying of this
code.

Possible now that we've dropped iOS 11 support; that was zulip#4664.
@chrisbobbe chrisbobbe deleted the pr-drop-ios-11 branch June 23, 2021 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants