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

Make the app compatible with Xcode 14 #7371

Merged
merged 6 commits into from
Sep 14, 2022
Merged

Conversation

crazytonyli
Copy link
Contributor

@crazytonyli crazytonyli commented Jul 28, 2022

Description

Here are the fixed required for the project to run on Xcode 14:

Testing instructions

No test required, as this PR goes to the xcode-14 branch.

@crazytonyli crazytonyli added the category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. label Jul 28, 2022
@crazytonyli crazytonyli added this to the 10.0 milestone Jul 28, 2022
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jul 28, 2022

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr7371-0405e14 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@crazytonyli
Copy link
Contributor Author

The test failures on this PR is a bit strange... UI tests on iPhone and iPad pass on my own mac, whereas they fail reliably on CI. Unit tests fail on my own mac too, but more than just the two cases on CI.

@crazytonyli crazytonyli marked this pull request as draft August 1, 2022 23:58
@crazytonyli
Copy link
Contributor Author

crazytonyli commented Aug 2, 2022

Similar to wordpress-mobile/WordPress-iOS#19119, I'll summarize the status of UI and unit tests. I'll come back once next beta is out.

@crazytonyli
Copy link
Contributor Author

the other is also formatter related. "lbs" isn't translated to localized "磅" in Chinese

This issue is fixed in beta 5.

@oguzkocer oguzkocer removed this from the 10.0 milestone Aug 11, 2022
@oguzkocer
Copy link
Contributor

@crazytonyli I am updating the milestone dates for WCiOS & WCAndroid. Since this PR is currently a draft, I removed the 10.0 milestone from it. I suggest waiting to add the milestone until you're ready to merge the PR. Sorry about the extra hassle.

@crazytonyli
Copy link
Contributor Author

@oguzkocer No problem. Thanks for the reminder. I should've removed the milestone when I turned this PR to draft.

@crazytonyli
Copy link
Contributor Author

the other is also formatter related. "lbs" isn't translated to localized "磅" in Chinese

This issue is fixed in Xcode 14 beta 6. Given Apple is about to release iOS 16 next week, I doubt the other two issues are going to be fixed in Xcode 14 GM. I'll start working on a fix.

@crazytonyli crazytonyli force-pushed the xcode-14-workarounds branch 5 times, most recently from cae7d48 to 284d890 Compare September 9, 2022 00:52
@itsmeichigo
Copy link
Contributor

Just commenting to note that this PR has changes that will also fix #7091, thanks @crazytonyli.

On iOS 16, `DateFormatter` returns a different localized string for template
format 'MMM d, h:mm a', compared to previous iOS versions.

Feeback to Apple: FB11335645.
@crazytonyli crazytonyli changed the title Use a Xcode 14 beta compatible fork of Charts Make the app compatible with Xcode 14 Sep 14, 2022
@crazytonyli crazytonyli requested a review from mokagio September 14, 2022 00:13
@crazytonyli crazytonyli self-assigned this Sep 14, 2022
@crazytonyli crazytonyli marked this pull request as ready for review September 14, 2022 00:13
@crazytonyli crazytonyli added this to the 10.4 milestone Sep 14, 2022
@crazytonyli
Copy link
Contributor Author

Hi @mokagio , this PR is ready for review. Thanks!

@mokagio
Copy link
Contributor

mokagio commented Sep 14, 2022

Spooky that the unit tests failed in CI. I retried them and they failed again, in the same way. Trying to replicate locally...

Copy link
Contributor

@mokagio mokagio left a comment

Choose a reason for hiding this comment

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

Third time lucky:

image

The flaky test is test_loading_indicator_gets_disabled_after_the_network_operation_completes. In the successful run, it took >2s to run. That it's so slow is a concern and something to investigate further.

image

@crazytonyli
Copy link
Contributor Author

@mokagio Thanks for getting the flaky test passed. #7719 should fix this flaky test. 🤞

@crazytonyli crazytonyli merged commit 92915ef into xcode-14 Sep 14, 2022
@crazytonyli crazytonyli deleted the xcode-14-workarounds branch September 14, 2022 06:19
@crazytonyli crazytonyli restored the xcode-14-workarounds branch September 14, 2022 22:54
@mokagio mokagio deleted the xcode-14-workarounds branch September 14, 2022 23:00
@crazytonyli crazytonyli mentioned this pull request Sep 15, 2022
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants