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

Fix podspec for coredata support #91

Closed
wants to merge 4 commits into from

Conversation

ryanmaxwell
Copy link

I had to make the following adjustments to the podspec for this to integrate into my project (the new mantle 2.0 branch)

Basically, the 'GCC_PREPROCESSOR_DEFINITIONS' => "OVERCOAT_SUPPORT_COREDATA=1" doesn't work when cocoapods creates the umbrella header for the swift modules without paying attention to the preprocessor macro to include the header implementation. So i'm left with an incomplete module.

I moved the source files into subdirectories that better align with the subspecs.

I also removed the MTLManagedObjectAdapter.h from the Overcoat.h header as that should be pulled in by the consumers where appropriate (like Mantle.h is). I added MTLManagedObjectAdapter as a dependency for the CoreData subspec, and also updated the versioning requirements to the '~>' syntax which is a better way to specify non-breaking newer versions of a pod are supported.

I am not actively using the social support in Overcoat so I don't think i've fixed that integration.

@sodastsai
Copy link
Member

I'll look into this later.

But one issue about the dependency is that I think it's better to keep both Mantle 1.x and 2.x supported, so this is why I wrote

ss.dependency 'Mantle', '<= 3.0'

instead of (in your PR)

ss.dependency 'Mantle', '~> 2.0'

And this is also why I didn't add MTLManagedObjectAdapter as default dependency.

TL;DR;
Any idea about to support only Mantle 2.x or both 1.x and 2.x?

@ryanmaxwell
Copy link
Author

Oh sure, I didn't realize that you wanted to keep support for both, but that's a noble goal. There must be a way to set it up to work across all configurations and versions but I didn't have time to test all the possible configurations and just needed to get it compiling today :)

I'll try and look into it more when I get a chance

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.

None yet

2 participants