-
Notifications
You must be signed in to change notification settings - Fork 470
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
Fix for swift 5.1 compiler warning when type is an enum with a none case #1482
Fix for swift 5.1 compiler warning when type is an enum with a none case #1482
Conversation
This looks good in theory but I want to test this against our existing codebase before I merge. I'm out moving until 3 September so it'll probably be around then, just FYI |
Yep, no problem. I saw that you were out, just thought I’d throw up the pull request anyway :) |
@designatednerd Have you had a chance to look at this? |
This will need some slight tweaks in light of #1515, which was just merged. Could you also add a line to the CHANGELOG for this as well? |
@designatednerd Updated it, but any advice on those CI failures. Seem to be completely unrelated. |
Those tests are super tempermental, I kicked both builds |
@cltnschlosser Mind pushing a dummy commit? Looks like our CI may have temporarily lost its mind and needs a kick at the push level. |
I pushed an empty file and that build passed, then I dropped that commit and the build failed :( |
:lolsob: Kicking the builds again to see if it's a transient failure - it's possible dropping the commit might have kicked things back to an old build where things got screwed up though. |
Yyyyyup. Looks like it needs another commit. I found a dumb thing it'd be nice to actually include so it doesn't have to be a total-dummy commit. |
🎉it worked! 🎉Now lemme test the branch against the SDK to figure out whether this needs to go as a patch or a minor - I suspect a patch but worth double checking... |
Patch it is! Merging |
Resolves apollographql/apollo-ios#724
cc @designatednerd