-
Notifications
You must be signed in to change notification settings - Fork 225
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 Carthage Support #21
Conversation
Any chance you'll be able to review this in the next few days, @kylef? I have a couple other changes queued up that I've held until this merges. Thanks! |
- xcodebuild -workspace JWT.xcworkspace -scheme JWT test -sdk macosx | xcpretty -c | ||
- pod lib lint |
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.
Can you restore this line, it's verifying if the pod spec is valid for CocoaPods.
@kylef I get the following error when I run
I've taken care of the other comments. |
# https://github.com/fastlane/fastlane/blob/master/docs/Gitignore.md | ||
|
||
fastlane/report.xml | ||
fastlane/screenshots |
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.
We don't use fast lane and a bunch of things in this gitignore, can you strip them out of this gitignore please?
Okay, just two remaining comments. I've fixed the issue with the podspec in 6df50e2, so rebasing on-top of master should solve this. Would you be able to address them and then squash these commits? I have one concern, that the Xcode project only has an OS X Target. From what I understand, that means Carthage will only be able to integrate this library into OS X and it won't work for iOS, tvOS and watchOS users. Is this a problem for you? |
I have another PR ready for iOS / tvOS targets that should take care of that. |
8e21f17
to
543fe2f
Compare
543fe2f
to
042be77
Compare
@kylef The pod lint step passes for me locally, but it looks like Travis has an issue with it. Any chance you can give me some guidance on that, since I'm not as familiar with what pods might be upset about? Thanks so much for your help with this. |
This change set provides Carthage support for the library, which would resolve #17.
I've attempted to follow the example of AlamofireImage for how to best satisfy dependencies while not requiring integrators to use a particular dependency management system. To accomplish this goal, I've removed internal use of Pods within JSONWebToken, and instead satisfy the CryptoSwift dependency with a submodule. I've added a Cartfile that handles the dependency chain for Carthage users, and attempted to verify CocoaPods support with a local spec. I've also updated the Travis tests and run them against my fork to make sure those continue working.
As a Carthage user, I'm most concerned about having possibly broken CocoaPods, so feedback in that area would be greatly appreciated.
As a look ahead, I'm also planning to submit changes to add support for an iOS framework target, as well as code changes to support custom header values and signing key formats. I'll also submit a follow-up PR with changes for the README.