-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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 template and migrate apps to iOS 11 minimum #101963
Conversation
@@ -759,7 +759,7 @@ Future<void> _writeIOSPluginRegistrant(FlutterProject project, List<Plugin> plug | |||
final List<Map<String, Object?>> iosPlugins = _extractPlatformMaps(methodChannelPlugins, IOSPlugin.kConfigKey); | |||
final Map<String, Object> context = <String, Object>{ | |||
'os': 'ios', | |||
'deploymentTarget': '9.0', | |||
'deploymentTarget': '11.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.
This is not used for published plugins, it's generated for app and modules that need to import plugins.
const String deploymentTargetOriginal9 = 'IPHONEOS_DEPLOYMENT_TARGET = 9.0;'; | ||
|
||
// Podfile changes. | ||
const String podfilePlatformVersionOriginal = "platform :ios, '9.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.
New Podfile
migration logic.
-platform :ios, '9.0'
+platform :ios, '11.0'
I can split the integration test/examples out into their own review if this is too many files, hopefully they are easy to review since those diffs are all the same, and very small. |
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
|
||
if (line.contains(deploymentTargetOriginal8) | ||
|| line.contains(deploymentTargetOriginal9) | ||
|| line.contains(podfilePlatformVersionOriginal)) { | ||
if (!migrationRequired) { |
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.
This is existing code, but I'm a little confused and try to understand this. Why do we still migrate if !migrationRequired
.
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.
Ahh, because .migrateLine()
is being for each line in processFileLines
, and if .migrateLine()
returns non-null (as it does in this codepath), it will set migrationRequired = true
, so that print will not show for subsequent lines.
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.
Exactly, the next line is // Only print for the first discovered change found.
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.
Ah ok. Sounds familiar, I probably asked the same question before and forgot about it.
When looking at the whole file, it makes sense but it might be a little confusing when doing inline reviewing like this.
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
1. Change templates to `IPHONEOS_DEPLOYMENT_TARGET`, `MinimumOSVersion`, and Podfile `platform :ios` to 12.0. 2. Add migrator for Podfile part to migrate `platform :ios, '11.0'` -> `platform :ios, '12.0'` 3. Compile with `-miphoneos-version-min=12.0` 4. Run the migrator on all example apps and integration tests. See also #62902 and #85174 and #101963 Fixes #136060
Reverts #140478 Initiated by: loic-sharma This change reverts the following previous change: Original Description: 1. Change templates to `IPHONEOS_DEPLOYMENT_TARGET`, `MinimumOSVersion`, and Podfile `platform :ios` to 12.0. 2. Add migrator for Podfile part to migrate `platform :ios, '11.0'` -> `platform :ios, '12.0'` 3. Compile with `-miphoneos-version-min=12.0` 4. Run the migrator on all example apps and integration tests. See also #62902 and #85174 and #101963 Fixes #136060
Reland #140478 with `ios_content_validation_test` test fix. ``` [ios_content_validation_test] Process terminated with exit code 0. Task result: { "success": true, "data": null, "detailFiles": [], "benchmarkScoreKeys": [], "reason": "success" } ``` __________ 1. Change templates to `IPHONEOS_DEPLOYMENT_TARGET`, `MinimumOSVersion`, and Podfile `platform :ios` to 12.0. 2. Add migrator for Podfile part to migrate `platform :ios, '11.0'` -> `platform :ios, '12.0'` 3. Compile with `-miphoneos-version-min=12.0` 4. Run the migrator on all example apps and integration tests. See also #62902 and #85174 and #101963 Fixes #136060
Reland flutter#140478 with `ios_content_validation_test` test fix. ``` [ios_content_validation_test] Process terminated with exit code 0. Task result: { "success": true, "data": null, "detailFiles": [], "benchmarkScoreKeys": [], "reason": "success" } ``` __________ 1. Change templates to `IPHONEOS_DEPLOYMENT_TARGET`, `MinimumOSVersion`, and Podfile `platform :ios` to 12.0. 2. Add migrator for Podfile part to migrate `platform :ios, '11.0'` -> `platform :ios, '12.0'` 3. Compile with `-miphoneos-version-min=12.0` 4. Run the migrator on all example apps and integration tests. See also flutter#62902 and flutter#85174 and flutter#101963 Fixes flutter#136060
IPHONEOS_DEPLOYMENT_TARGET
,MinimumOSVersion
, and Podfileplatform :ios
to 11.0.platform :ios, '9.0'
->platform :ios, '11.0'
-miphoneos-version-min=11.0
Note this does not change the plugin minimum support version, tracked separately in #101961
See also #62902 and #85174
Fixes #101960
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.