Skip to content
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 #55

Merged
merged 8 commits into from
May 17, 2016

Conversation

mattprowse
Copy link
Contributor

I've been happily using LetsMove in all of my Mac apps for years, and since I'm using Carthage to manage dependencies these days I thought it would be useful to add support for installing LetsMove using Carthage.

Adding Carthage support basically just involves adding a shared Framework target to the project (plus an umbrella header and a module map for the framework), which does unfortunately mean there are a lot of messy changes in the project file.

Additionally, since it makes sense for the shared framework to be named "LetsMove", I've renamed the test app target "LetsMove Test".

I've tried to change as little as possible to get this working, but I'm happy to make any changes necessary to help get this merged.

@andypotion
Copy link
Collaborator

andypotion commented May 16, 2016

I notice that deployment target has been changed to 10.6 from 10.5. Would it be possible to keep it at 10.5? Otherwise, we're going to have to update the version and note the change in the readme.

@mattprowse
Copy link
Contributor Author

Oh, sorry I completely forgot to mention that change! I set the deployment target for the framework target to 10.6 to avoid a linker warning:

(/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/arc/libarclite_macosx.a(arclite.o)) was built for newer OSX version (10.6) than being linked (10.5)

ARC is disabled on the framework target but that doesn't seem to stop Xcode from linking with libarclite unfortunately. My guess is that building for deployment on 10.5 is no longer officially supported when you're building against the 10.11 SDK with Xcode 7.

That said, other than the warning there isn't really any need to change the build target. And the warning doesn't seem to cause any problems, so it's probably not a big deal to just change change the deployment target back and put up with the warning being there.

Let me know which way you want to go and I'll update the PR with the changes ASAP.

@andypotion andypotion merged commit dcd473a into potionfactory:master May 17, 2016
@andypotion
Copy link
Collaborator

Having slept on it, it seems best that we do drop 10.5 support. Even with just 10.6, we're supporting 6 OS releases. I've updated the version to 1.21 and published to master. Thanks for the PR.

@mattprowse
Copy link
Contributor Author

Great, thanks for reviewing this so quickly! Thanks too for writing LetsMove in the first place. As I say I've been using it for years in my apps; it's one of a handful of essentials that I use in every Mac app I write.

@mattprowse
Copy link
Contributor Author

Sorry to be a pain, but any chance you could add a tag and a release for version 1.21? Installing from Carthage won't work without them. Thanks!

@andypotion
Copy link
Collaborator

I pushed a v1.21 tag. But what do you mean by "release for version 1.21?"

@mattprowse
Copy link
Contributor Author

Sorry, that's just my ignorance about how GitHub works. For some reason I thought releases and tags were separate. Everything looks to be working fine now that the v1.21 tag is there. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants