-
Notifications
You must be signed in to change notification settings - Fork 68
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
Changes for Swift3 #49
Conversation
This failure seems to be because its trying to build with xcode7.1, this needs to be built with 8.0. Let me know if I need to update something. |
Test is failing because it is trying to run it on iOS 10 simulation and as I mentioned it will fail because of the issue #47. I need help to update config in way to make it not run on ios10. Please help. |
I assume this is now actually good enough to be merged. Unfortunately, I will not be able to push the new release to cocoapods in a while, but I could still merge this. Thoughts? |
@rajeevpra I don't think it makes sense to run the tests on an outdated version of iOS, as that's not really proving that the code works. You can make these tests work in iOS 10 by creating a dummy app target with Keychain Sharing enabled, and setting the tests to run "On Host App" with that app. |
I agree @jorystiefel. I will give it a try when I get chance. It can be argued that it is still useful for people to have it as this only affects ios 10 simulator not the device. |
Another note I feel worth mentioning: It seems this branch of Heimdall is not backwards compatible with the current master. We could not get our client/server to successfully talk to each other until both were on the Swift 3 branch. |
I will be unavailable to discuss/test/merge this for a while as I am away from my dev machine and lack the necessary tools. Two things to note:
|
One last thing, based on this discussion, I am willing to give someone else merge rights to master, as well as cocoapods. This is to speed up the process of adding Swift 3 support. @jorystiefel would you be interested in this? |
@henrinormak Sure, I'd be happy to help. |
Definitely looking to use the swift 3 version soon! I am using it to add RSA to https://github.com/kylef/JSONWebToken.swift. Currently I'm having issues pointing the pod dependency to @rajeevpra's repository and it seems like having it is due to only being able to use official pod releases. |
Added @jorystiefel as a collaborator to the repo, I would also add you to the cocoapod as an owner, but I need your email for that |
@henrinormak thank you, my email is jorystiefel at me.com |
Have you registered that email with Cocoapods? It claims that you have not, if so, could tou, so that I could also add you as a contributor there |
All set. |
Done on my end, I'll leave merging/testing/verifying this up to you. Once done please also push a new release to Cocoapods |
@rajeevpra @rchatham I'm going to merge this soon. I've been testing this extensively over the past week. The break in compatibility I noticed was due to the padding change that happened in April, not due to the Swift 3 changes. I will make updates to make the tests work on iOS 10 simulator after this is merged. |
is it available in cocoapods yet? |
I don't think it is, @jorystiefel could you also update the pod? You need to update the podfile withnew version number, but that's pretty much it. |
Yes, coming today. |
thanks for doing this! much appreciated =) |
Please |
@rchatham I'm having problems with CocoaPods crashing, sorry |
What's wrong? Have you tried running pod lint? |
From what I can find you will need to add a script like the one below, https://github.com/iosdevzone/IDZSwiftCommonCrypto/blob/master/GenerateCommonCryptoModule.swift And run it in the .podspec. https://github.com/iosdevzone/IDZSwiftCommonCrypto/blob/master/IDZSwiftCommonCrypto.podspec It gets mentioned it in the beginning of the realm talk here https://realm.io/news/danny-keogan-swift-cryptography/. |
@rchatham I followed your guidance, it fixed the CommonCrypto issue |
@jorystiefel The build from cocoapods is broken. Have you merged the PR? |
Most of the changes are created automatically by xcode.. I had fix few issues after that. I ran all the tests successfully. Please make sure to have simulator is set to ios 9.x. It will fail on 10.x because of bug in new simulator.