woocommerce-ios icon indicating copy to clipboard operation
woocommerce-ios copied to clipboard

Published 20 hours ago • verified publisher icon woocommerce

Use a Xcode 14 beta compatible fork of Charts

Open crazytonyli opened this issue 2 years ago • 4 comments

Description

The Charts library doesn't compile on Xcode 14. This PR switches to a fork of the Charts library. This change is meant to be temporary, which shouldn't land on the trunk branch. There are a couple PR open in the official repository to address this. I'll keep an eye on the repository and revert this change once the official library supports Xcode 14.

Testing instructions

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

crazytonyli avatar Jul 28 '22 04:07 crazytonyli

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.

wpmobilebot avatar Jul 28 '22 04:07 wpmobilebot

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 avatar Aug 01 '22 10:08 crazytonyli

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

  • Two unit tests fail on CI:
    • one failure due to a DataFormatter bug in Xcode beta. "Monday, Jan 3 at 1:00 AM" is returned when using EEEE, MMM d, h:mm a format
    • the other is also formatter related. "lbs" isn't translated to localized "磅" in Chinese
  • UI tests fail due to https://github.com/woocommerce/woocommerce-ios/issues/7393

crazytonyli avatar Aug 02 '22 01:08 crazytonyli

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

This issue is fixed in beta 5.

crazytonyli avatar Aug 10 '22 09:08 crazytonyli

@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.

oguzkocer avatar Aug 11 '22 19:08 oguzkocer

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

crazytonyli avatar Aug 11 '22 21:08 crazytonyli

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 avatar Aug 30 '22 01:08 crazytonyli

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

itsmeichigo avatar Sep 12 '22 11:09 itsmeichigo

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

crazytonyli avatar Sep 14 '22 00:09 crazytonyli

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

mokagio avatar Sep 14 '22 03:09 mokagio

@mokagio Thanks for getting the flaky test passed. https://github.com/woocommerce/woocommerce-ios/pull/7719 should fix this flaky test. 🤞

crazytonyli avatar Sep 14 '22 06:09 crazytonyli