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
Set target deployment to iOS 15.0 #1878
Conversation
@@ -112,10 +112,8 @@ public final class LoginToutViewController: UIViewController, MFMailComposeViewC | |||
_ = self.backgroundImageView | |||
|> backgroundImageViewStyle | |||
|
|||
if #available(iOS 13.0, *) { |
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.
Phew, now these are some old checks!
return ChangeEmailViewController.instantiate() | ||
} | ||
let changeEmailView = ChangeEmailView() | ||
return UIHostingController(rootView: changeEmailView) |
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.
Do we want to clean up/delete the old ChangeEmailViewController
, or leave it until SwiftUI is no longer 'experimental'? The new ChangeEmailView
is definitely missing some tests, for instance, but it's not like this was behind a feature flag...
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 think we could get rid of it now or separately. We had just kept it for iOS 14 and to have it as a reference while we played with SwiftUI.
Codecov Report
@@ Coverage Diff @@
## main #1878 +/- ##
==========================================
+ Coverage 83.95% 83.97% +0.01%
==========================================
Files 1222 1222
Lines 111082 111058 -24
Branches 29522 29505 -17
==========================================
- Hits 93261 93257 -4
+ Misses 16807 16789 -18
+ Partials 1014 1012 -2
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
guard #available(iOS 14.5, *), supportedModels.contains(ProcessInfo().environment[modelKey] ?? "") else { | ||
fatalError("Please only test and record screenshots on an iPhone 8 simulator running iOS 14.5") | ||
guard #available(iOS 15, *), supportedModels.contains(ProcessInfo().environment[modelKey] ?? "") else { | ||
fatalError("Please only test and record screenshots on an iPhone 8 simulator running iOS 15.5") |
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.
Note that CircleCI was already testing against 15.5, so no updates to snapshots are needed
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.
LGTM 😄
There's actually one more #available(iOS 15, *)
in TestCase.swift
line: 94 that we could clean up
return ChangeEmailViewController.instantiate() | ||
} | ||
let changeEmailView = ChangeEmailView() | ||
return UIHostingController(rootView: changeEmailView) |
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 think we could get rid of it now or separately. We had just kept it for iOS 14 and to have it as a reference while we played with SwiftUI.
Whoops, good catch! I'll grab that one. |
f4998ea
to
01730ae
Compare
📲 What
This drops support for iOS 14 and makes our minimum target iOS 15.
🤔 Why
We only want to support three iOS versions at a time - so now we have 15, 16 and 17.
✅ Acceptance criteria
Regression Tests
PledgeViewController