-
Notifications
You must be signed in to change notification settings - Fork 433
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
Add primary associated types to SignalProducerConvertible & SignalProducerProtocol #855
Add primary associated types to SignalProducerConvertible & SignalProducerProtocol #855
Conversation
This breaks compilation with older compiler versions though. |
Ahhh, I was afraid of that. Let me see if I can handle that better 🤔 |
Made a first pass at this. I'm not going to lie, I don't love it. Requires a bunch of repeated code... Looks like the original proposal had the concept of a |
I think this could work. We need to make sure CI tests run on Xcode 14 to test this too. |
Tweaked the test.yml to run on both macOS-12 and macOS-11. My GitHub Actions knowledge is a little limited so let me know if I did anything dumb. |
Just following up on this. @NachoSoto @mluisbrown is there anything you need me to do to help get this merged |
@braker1nine we should try and have CI passing. There are issues with macCatalyst but the rest should pass ok. |
Oh sheesh. I missed that. Sorry @mluisbrown! |
I made a bit of a tweak to the testing structure on this one to try to increase the coverage of Swift versions
|
What do you think of opening a separate PR with the CI changes? We can merge that right away then I'll take a look at this one 🙏 |
@NachoSoto we also need to address the fact that Mac Catalyst CI jobs are simply not working. IMO we should just disable the Mac Catalyst job until we have a better idea of what the issue is, as it's going to cause issues with all PRs. What happens is that the job starts and then just hangs for ever until GitHub Actions decides to cancel it (hours later). If I run the exact same The issue is affecting this PR and also this one and affected this one too. @p4checo tried using Xcode 14.1 in his PR to see if it made a difference and it didn't 🤷♀️ |
Yeah I can absolutely do that.
@mluisbrown Would it be worth disabling Catalyst in that same PR? |
@braker1nine yes, absolutely 👍 |
Opened up #858. I didn't disable Catalyst in there, but I could add that |
f378d11
to
a4f4f7f
Compare
Adds primary associated types to
SignalProducerConvertible
andSignalProducerProtocol
Checklist