-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[camera] Update and migrate iOS example project #2090
Conversation
Does a change to the example app require a build suffix version bump? |
12050a1
to
5f6a8d7
Compare
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 pending one nit on the versioning bump and a test exemption.
Lately we've re-emphasized that all PRs need tests, following the Flutter style guide. This PR is already being effectively tested by our existing infra because it's a change to the example app's iOS config, and our infra make sure that the example app still builds on iOS. I don't think there's a meaningful test to be added here on top of what we already have. @Hixie can we get an exemption for this one and the other build refactoring patches @jmagman is landing?
packages/camera/CHANGELOG.md
Outdated
@@ -1,3 +1,7 @@ | |||
## 0.5.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.
nit: This should be 0.5.4+3
. Pub changes it's semver scheme for when the plugin is above or below 1.0.0. When it's above 1.0.0 it's major.minor.patch
but when it's below it's 0.major.minor+patch
.
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 for the catch, sorry about that!
These changes get us to a baseline where the iOS example apps are migrated and build, so I can start more easily adding tests without including all these diff lines in a PR for every plugin, mostly to make those future reviews smaller and more focused. |
I talked with @Hixie offline. The I still think we can't land that exactly in this PR because other plugins are failing so CI would go red. But @jmagman either you or I can land it as a followup to these fixes to prevent this from regressing. I can add it in myself once these are landed. |
Better yet, it shouldn't be present at all. Let me fix that in this change. |
I don't see how anything could be testing this builds. Here's what it looks like on master:
|
The Cirrus task is here: https://github.com/flutter/plugins/blob/master/.cirrus.yml#L76-L78. That goes through a couple layers, but it ends up running |
Ah, it's because it runs the migrator every time. Note the "Removing obsolete reference to flutter_assets from Runner.app" line.
|
1. Let flutter/flutter#26630 flutter_assets migrator run 2. Run `pod install` so flutter_assets is removed from the asset copy build phase 3. Migrate deprecated "English" to "en" language 4. Allow Xcode to remove extraneous xcconfigs, see flutter/flutter#38724 5. Let Xcode 11 update build settings 6. Remove DEVELOPMENT_TEAM references since having those provisioning profiles should not be required to run the examples (most examples don't have one) 7. Looks like this was last run with `use_frameworks!`? Let CocoaPods build as libraries instead of frameworks. 8. Remove ARCHS, which was causing a compilation error
1. Let flutter/flutter#26630 flutter_assets migrator run 2. Run `pod install` so flutter_assets is removed from the asset copy build phase 3. Migrate deprecated "English" to "en" language 4. Allow Xcode to remove extraneous xcconfigs, see flutter/flutter#38724 5. Let Xcode 11 update build settings 6. Remove DEVELOPMENT_TEAM references since having those provisioning profiles should not be required to run the examples (most examples don't have one) 7. Looks like this was last run with `use_frameworks!`? Let CocoaPods build as libraries instead of frameworks. 8. Remove ARCHS, which was causing a compilation error
1. Let flutter/flutter#26630 flutter_assets migrator run 2. Run `pod install` so flutter_assets is removed from the asset copy build phase 3. Migrate deprecated "English" to "en" language 4. Allow Xcode to remove extraneous xcconfigs, see flutter/flutter#38724 5. Let Xcode 11 update build settings 6. Remove DEVELOPMENT_TEAM references since having those provisioning profiles should not be required to run the examples (most examples don't have one) 7. Looks like this was last run with `use_frameworks!`? Let CocoaPods build as libraries instead of frameworks. 8. Remove ARCHS, which was causing a compilation error
Description
pod install
so flutter_assets is removed from the asset copy build phaseuse_frameworks!
? Let CocoaPods build as libraries instead of frameworks.Related Issues
Get to a buildable baseline so I can start working on flutter/flutter#41007.
Checklist
///
).flutter analyze
) does not report any problems on my PR.Breaking Change