-
Notifications
You must be signed in to change notification settings - Fork 217
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
Modernize CI #300
Modernize CI #300
Conversation
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.
Thank you for the update!
Unit tests failing due Xcode 12 hanging is likely the same as this issue |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #300 +/- ##
==========================================
- Coverage 86.81% 80.72% -6.10%
==========================================
Files 16 15 -1
Lines 986 887 -99
==========================================
- Hits 856 716 -140
- Misses 130 171 +41 |
@dfed Are you okay dropping any versions of Xcode in the CI matrix? Or do you want to keep going with Xcode 11? |
I'd hate to lose building for macOS 10.15 😕. That said... we are a bit long overdue for a breaking version bump. Could be nice to drop support for watchOS 2-3, tvOS 9-11, iOS 9-11, and macOS 10.11-10.13. Only issue there is we can't set our minimum Xcode version to something recent and still test older simulators in CI. Regarding the current failing tests... seems like tests protected by |
I hoped that those failures were dealt with in the existing CI infrastructure, but I guess not. I'll take a look. |
I can keep it, but technically that image is deprecated by GitHub, so I don't know how much longer it'll exist at all. |
Thank you! In the past we've avoided running unit tests that required entitlements in CI. We may need to take the same approach here, at which point we'd rename
Oh! That's information I didn't have. If those images are indeed deprecated and slated for removal then yeah we can remove them. cc @efirestone |
What's the recommended way to test locally with the proper entitlements? |
Our Contributing.md file outlines how to test with local entitlements on macOS – if iOS now requires testing on device as of Xcode 14, you'll need to go through similar steps on iOS. Though note that the iOS |
@dfed I updated the condition and the tests pass locally without entitlements, so I'd like to try it here before I full rename and add all the additional CI runs. |
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.
Really appreciate your progress here.
Odd, I pushed changes but they aren't being reflected here. GitHub issues I guess, but I think we're at a good point once those changes are in. I moved the Carthage and CocoaPods runners to use Xcode 14.3, do you want some dedicated Xcode 11 tests instead? |
Ah, looks like Xcode 14.3 removed |
What you've done here makes sense to me. However, it seems our supporting absurdly old iOS versions is causing our Carthage build to fail with modern Xcode. Oof. I am leaning towards ripping off the bandaid and rolling a major version bump for this. Let's drop support for anything prior to iOS 11, tvOS 11, watchOS 4, and macOS 10.13 and update the README to reflect that. I'm also seeing an error that
I think that'll do it given a similar script I have in another repo. |
One other wrinkle I recall is that |
welp. I'd be fine moving the xcodeproj/xcworkspace or Package.swift into a new directory. I think I'd prefer moving the |
I don't think it can work any other way, given Xcode / SPM doesn't support nested package files AFAIK. |
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.
Looking good!
Co-authored-by: Dan Federman <[email protected]>
Thanks for the work here @jshier! Merging 😄 |
This PR starts modernizing the project's CI integration, starting with updating the platform support.