-
Notifications
You must be signed in to change notification settings - Fork 118
[WCiOS17] Bump minimum deployment target to iOS17 #16003
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
Conversation
|
Only 1 review is 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.
Thanks!
All these repeating changes suggest that there should be another part of the code that switches from 16.0 -> 17.0 in a more centralized place.
In this case I think are two files that need to be updated:
- /config/Common.xcconfig
- Package.swift
There comes from the new apps infra created module infrastructure.
Generated by 🚫 Danger |
Thanks for the review! Interestingly, the Addressed here: 45730e4 |
I did resolve these in Xcode after updating the config, but there was no updates/changes to commit, so we should be good to go here. @itsmeichigo if you have the time this week could you please take a look into this one? As Povilas will be AFK the rest of the week and I'll be AFK next week 😅 If you don't have the time that's fine, no rush! 🙇 |
@@ -18948,6 +18971,7 @@ | |||
CLANG_ENABLE_OBJC_WEAK = YES; | |||
CODE_SIGN_STYLE = Automatic; | |||
INFOPLIST_FILE = WooCommerceScreenshots/Info.plist; | |||
IPHONEOS_DEPLOYMENT_TARGET = 17.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.
@iamgabrielma I think all these should be reverted, since they are probably overriding the common config file.
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.
Ooof you're right, sorry for the back and forth. Confirmed that after reverting on d50f5ee that we still see the targets individually marked as 17.0 under General
and builds normally. So it would seem that after the infra changes the only needs are, as you mentioned:
/config/Common.xcconfig
/Modules/Package.swift
This reverts commit 0ec09b8.
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.
The app builds and runs successfully. Project settings all display the iOS 17 as the target deployment.
Seems we're blocked from merging until you approve it @staskus ! 😂 ![]() I'll leave this one in auto-merge for now, no rush! |
Closes WOOMOB-1000. Part of WOOMOB-1003
Description
This PR bumps WooCommerce iOS minimum deployment target to iOS17.
Testing information
Tested on iPhone 14 running 17.6.1, and simulators running 18+
Screenshots
N/A
RELEASE-NOTES.txt
if necessary.