Skip to content

Conversation

@jtreanor
Copy link
Contributor

@jtreanor jtreanor commented Apr 1, 2019

This fixes compatibility with Xcode 10.2 by updating Aztec, updating Zendesk and fixing the new Xcode warnings/errors.

Related pull requests and issues:

Still to do:

Note: Due to the Zendesk SDK update this breaks compatibility with Xcode 10.1. We should merge this very carefully with plenty of notice.

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@jtreanor jtreanor added this to the 12.2 milestone Apr 1, 2019
@jtreanor jtreanor force-pushed the cocoapods-1-6-xcode-10-2 branch from ec5dd41 to 2adf266 Compare April 1, 2019 11:45
@wordpress-mobile wordpress-mobile deleted a comment from houndci-bot Apr 1, 2019
@wordpress-mobile wordpress-mobile deleted a comment from houndci-bot Apr 1, 2019
@jtreanor jtreanor force-pushed the cocoapods-1-6-xcode-10-2 branch from 2adf266 to c3a2b4e Compare April 1, 2019 14:37
@jtreanor jtreanor modified the milestones: 12.2, 12.3 Apr 1, 2019
@jtreanor jtreanor force-pushed the cocoapods-1-6-xcode-10-2 branch 6 times, most recently from 2113165 to debb071 Compare April 8, 2019 09:04
@jtreanor jtreanor mentioned this pull request Apr 10, 2019
1 task
@jtreanor jtreanor force-pushed the cocoapods-1-6-xcode-10-2 branch 6 times, most recently from 27b7e88 to 3d318ba Compare April 11, 2019 09:46
@jtreanor jtreanor added Tooling Build, Release, and Validation Tools and removed [Status] Not Ready for Review labels Apr 11, 2019
@jtreanor jtreanor marked this pull request as ready for review April 11, 2019 10:14
@jtreanor jtreanor requested a review from jkmassel April 11, 2019 11:52
@jkmassel
Copy link
Contributor

Looks like a Podfile.lock conflict has popped up in this one

@jkmassel
Copy link
Contributor

jkmassel commented Apr 11, 2019

This builds cleanly, which is great.

However, I'm seeing a few issues we probably want to address:

  • React Native is missing nullability hints in Objective-C, which is causing compiler warnings
  • WPMediaPickerViewController is missing nullability hints in Objective-C, which is causing a single compiler warning
  • We need to upgrade ZenDesk to use the non-deprecated APIs in order to avoid compiler warnings.

Typically we don't accept a merge that still has these warnings, but it's also possible that our dependencies may not get to this right away.

WDYT?

Screen Shot 2019-04-11 at 11 48 33 AM

@jtreanor
Copy link
Contributor Author

Thanks for taking a look, Jeremy! I completely agree that we should avoid introducing warnings.

There are two things happening here:

  1. I should have been clearer but as I mentioned in the PR description, given that I do not have all the context and past experience of working with the Zendesk SDK, I was hesitant to migrate to the new API. My preference would be to open an issue for this when merged. Happy to discuss though.
  2. We are experiencing an issue with CocoaPods + Xcode 10.2 where inhibit_all_warnings! is not working correctly for header files (inhibit_all_warnings! not work for header files in CocoaPods v1.6.0.beta.2 CocoaPods/CocoaPods#8219). I saw this but mistakingly thought it had been solved (they went away for me 🤷‍♂️).

I believe it should be possible to work around the CocoaPods warnings issue so I can definitely take a look at doing that. I think it may just be a wrong project setting (Its not happening for Woo). Unfortunately, it won't be realistic to fix the actual RN warnings.

@jtreanor jtreanor force-pushed the cocoapods-1-6-xcode-10-2 branch from 3d318ba to b70ab95 Compare April 15, 2019 09:57
@jtreanor jtreanor force-pushed the cocoapods-1-6-xcode-10-2 branch 2 times, most recently from 8106579 to 0bc956c Compare April 15, 2019 10:59
@jtreanor jtreanor force-pushed the cocoapods-1-6-xcode-10-2 branch from 0bc956c to e22ec3d Compare April 15, 2019 11:59
@jtreanor
Copy link
Contributor Author

@jkmassel I have tried several things to suppress the nullability warnings, but so far nothing has worked. I also cannot reproduce the issue in any other project.

I have opened wordpress-mobile/MediaPicker-iOS#320 to at least eliminate the warning we have control over.

@jkmassel
Copy link
Contributor

Fair enough! Hopefully RN gets them fixed soon! :)

Copy link
Contributor

@jkmassel jkmassel left a comment

Choose a reason for hiding this comment

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

Per our discussion – even if we can't get the third-party warnings fixed just yet, I'm good to :shipit:

@jtreanor jtreanor force-pushed the cocoapods-1-6-xcode-10-2 branch from 58a5d69 to 74e5b05 Compare April 16, 2019 09:19
@jtreanor
Copy link
Contributor Author

Resolved the conflicts with no other changes so I will go ahead with the merge.

I was still unable to suppress the warnings. I have opened a PR for React Native to fix them here: facebook/react-native#24467.

@jtreanor jtreanor merged commit a93cb6c into develop Apr 16, 2019
@jtreanor jtreanor deleted the cocoapods-1-6-xcode-10-2 branch April 16, 2019 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Tooling Build, Release, and Validation Tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants