-
Notifications
You must be signed in to change notification settings - Fork 171
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
Carthage fix #149
Carthage fix #149
Conversation
- Added CODE_SIGNING_REQUIRED = NO; CODE_SIGN_IDENTITY = ""; to tests
- Made frameworks optional - Added alternative Carthage framework - Updated Carthage if then script check
- Changed to copy framework to expected Pods_GEOSwift.framework name
Okay so I have the tests running again but two are failing:
Is it possible something has changed in QuickLook? |
I'd check to make sure the tests are still running on "iPhone XS" simulator. The snapshot tests are simulator-dependent for some reason. |
@macdrevx 😅Looks like it worked. 😎 |
@macdrevx Anything preventing this from being merged? |
I just need some time to review it. Sorry for the delay. |
@madhavajay I pushed a branch https://github.com/GEOSwift/GEOSwift/tree/carthage with a different approach. Can you try it out and let me know if it works for you? Instead of working around CocoaPods, I switched the project to use Carthage for development purposes. CocoaPods is still available for consumers. This seems simpler to me. The only problem left that I'm aware of is that I get an error when I try running the playground.
Do you have any ideas on how to get it working again? Regarding number 3 in your PR description: I saw that error too, but it was solved by a change to the scheme to prevent the tests from being compiled unless we're actually building for test. |
Yes, I think using Carthage is a nicer solution than the above hack but I wasn't sure you would want to overhaul the whole dependency management system. I have added a PR which fixes the Playground issue. Let me know if you have any issues with it! :) |
Closing this in favor of #153. |
Okay so this time its much better.
This relies on the PR over at geos:
GEOSwift/geos#5
We shouldn't compile the library for Production with @testable enabled so this should be done as part of an App Target or its own Test Target, but not attached to the Framework directly.
It seems to work great! :)
Also, it should be pretty easy to add the additional tvOS and macOS targets to this library if you wish now that they are part of the geos dependency.
Thoughts?