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

breaking: upgrade xcode compatible to 11.0 #780

Merged
merged 1 commit into from
Feb 14, 2020

Conversation

erisu
Copy link
Member

@erisu erisu commented Feb 10, 2020

Motivation and Context

Upgrade project, CordovaLib, and CordovaLib test to be Xcode 11.0 compatible.

Even though next major will support iOS 11 minimum, which was introduced in Xcode 9.x, the goal of this PR is to set the version of Xcode that Apple is enforcing all apps to be built with for app submission starting April 2020.

Keeping it modern, or as modern as possible with our target release.

WWDC 2019 Video, as Apple says, around 6:15 mark:

Keeping your project file modern is a critical way to make sure that Xcode can help you out and avoids the accumulation of issues.
First, when you're updating to a new version of Xcode, you'll be offered the opportunity sometimes to have Xcode update the project settings and update your project file to the latest format.

Description

  • Change the Project Format to Xcode 11.0-compatible.
  • Rebuilt the SampleApp's project.pbxproj with the current PR's changes.
  • Ensure that values in project.pbxproj that contains __PRODUCT_NAME__ has quotations.

Testing

  • npm t
  • cordova platform add
  • cordova build ios

Checklist

  • I've run the tests to see all new and existing tests pass
  • I've updated the documentation if necessary

@erisu erisu added this to the 6.0.0 milestone Feb 10, 2020
@erisu erisu requested review from dpogue and jcesarmobile February 10, 2020 06:27
@codecov-io
Copy link

codecov-io commented Feb 10, 2020

Codecov Report

Merging #780 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #780   +/-   ##
=======================================
  Coverage   74.26%   74.26%           
=======================================
  Files          11       11           
  Lines        1815     1815           
=======================================
  Hits         1348     1348           
  Misses        467      467

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59d0dd7...edf870f. Read the comment docs.

@erisu erisu force-pushed the breaking/upgrade-xcode-compatible branch from 66eb78d to edf870f Compare February 10, 2020 11:26
@erisu erisu marked this pull request as ready for review February 10, 2020 11:27
@jcesarmobile
Copy link
Member

Why Xcode 9? Apple requires Xcode 10 for submissions, and soon (by April) they will require Xcode 11.
So maybe we can target Xcode 11 already and remove Xcode 10 and earlier from tests

@erisu
Copy link
Member Author

erisu commented Feb 10, 2020

I forgot all about the new submission rules from Apple. I will go ahead and update to Xcode 11 to follow the submission rule which is described on this page.

https://developer.apple.com/app-store/submissions/

Starting April, 2020, all apps submitted to the App Store will need to be built with Xcode 11. Xcode 11 requires macOS Mojave 10.14.3 or later.

@erisu erisu changed the title breaking: upgrade xcode compatible to 9.3 breaking: upgrade xcode compatible to 11.0 Feb 10, 2020
@erisu erisu force-pushed the breaking/upgrade-xcode-compatible branch 3 times, most recently from 6f438ef to fb95342 Compare February 10, 2020 13:27
@BBosman
Copy link
Contributor

BBosman commented Feb 10, 2020

I would have expected the “LastUpgradeCheck” to reference the XCode 11 level as well?

@erisu
Copy link
Member Author

erisu commented Feb 11, 2020

The changes in this PR were made though the Xcode interface. Nothing was manually changed except to re-add back quotation wrappers around the product name for UTF-8 support. I believe LastUpgradeCheck shouldn't be manually changed.

Xcode should have updated it if it was needed. I am guessing that Xcode did not change LastUpgradeCheck because it was not triggered by an upgrade notification.

If we manually changed it, we would need to know the correct value.

  • What would be the side-effect of changing it manually?
  • If not everything was upgraded and it was manually changed, would it be basically be disabling the flag that Xcode might used to determining to display an upgrade notification? And if disabled and if there were missed changes, would they not be corrected?

Anyways, I think it is safer to keep it as is and wait till Xcode automatically updates this for us.

Actually, I have another PR that I am preparing that did update LastUpgradeCheck though the Xcode UI. Those changes will not be merged with this PR and will be treated as a separate change.

@erisu erisu force-pushed the breaking/upgrade-xcode-compatible branch from fb95342 to 54244e3 Compare February 13, 2020 01:54
@erisu
Copy link
Member Author

erisu commented Feb 14, 2020

Last call, this PR will be merged with or without a review in 24 hours.

@erisu erisu requested review from shazron and knight9999 February 14, 2020 05:43
Copy link
Contributor

@purplecabbage purplecabbage left a comment

Choose a reason for hiding this comment

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

lgtm!

@erisu erisu merged commit 87aeed9 into apache:master Feb 14, 2020
@erisu erisu deleted the breaking/upgrade-xcode-compatible branch February 14, 2020 11:41
NiklasMerz pushed a commit to GEDYSIntraWare/cordova-ios that referenced this pull request Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants