Added Carthage Support #164
Conversation
This is a completel reorganization of the project, but there shouldn't be any changes to the actual code.
This should have been included with the Carthage restructuring.
|
@davbeck this looks great, and I'm reviewing it today/tomorrow as I find time. As a first small thing to fix, it looks like FLAnimatedImage is included in the NYTPhotoViewer project with an absolute path instead of "Relative to Group":
Can you please push that small change to your branch, and I'll keep looking? |
|
Good catch. It opens up another issue though I wasn't aware of. At least on my Mac with Xcode 7.2.1 (7C1002), linking against the relative framework doesn't properly add the project as a dependency to the Example app and the projects from the workspace don't appear when adding dependencies manually. I don't normally use workspaces with Carthage (it's not how the official Carthage docs recommend setting up projects) but I wanted to follow the ThoughtBot guide linked to in the issue. |
... by removing the workspace. I'm not entirely sure if this was due to a bug in Xcode, intended behavior, a bad system configuration, or user error, but the target dependencies were not working between projects in the same workspace. However, projects can be linked from other projects without an explicit workspace. Switching to this method resolved any issues I was seeing.
|
I think I've resolved that issue in the last commit by removing the workspace and just using a single project. I also did a |
When we added Carthage support ( #164 ), we created two NYTPhotoViewer targets: the “normal” one which includes animated gif support (via `ANIMATED_GIF_SUPPORT=1`, and requires linking with FLAnimatedImage); and the “-Core” target which does not include animated GIF support. For brevity I will refer to the library version with animated gif support as “-Normal” even though it doesn’t have that suffix (the “-Core” target _does_ have that suffix). The unit tests are configured to run against the “-Normal” target, but they weren’t compiled with `ANIMATED_GIF_SUPPORT=1`, so we were running unit tests intended for “-Core” against the “-Normal” target. This resulted in some test failures. I’ve fixed them by removing the `ANIMATED_GIF_SUPPORT` conditionals from the test target, leaving in the code that should be run against “-Normal”. I am unsure whether it’s worth making a new “-Core” test target just to include the three assertions (removed in this commit) which were intended to apply to “-Core”. Feedback is welcome.
This is done around the requirements and desires outlined in #61, meaning that it is a complete project restructuring.
I've included the framework in my own app using the following and so far everything is working as expected:
See my note on the issue in regards to GIF support. This is, in my opinion, the closest analogue to Cocoapods subspecs.
As for the resource bundle, I just made the folder a bundle itself. CocoaPods should create it's own bundle based off of the files in that folder/bundle and Carthage should recognize the folder as a bundle and embed it as such. No need to special case any code.
I did run
pod lib lint, but I haven't tried to pull in the framework using CocoaPods just yet.