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

Allow Swift static library integration via CocoaPods #570

Merged
merged 3 commits into from
Jul 31, 2019

Conversation

chrisballinger
Copy link
Contributor

@chrisballinger chrisballinger commented Jul 17, 2019

Pull Request Checklist

  • This pull request is submitted against the development branch.
  • Inline documentation has been updated.
  • README.md or another dedicated guide has been updated.
  • Changes are tested.

This PR resolves issue #360 which prevented using GRDB when integrated as a Swift static library via CocoaPods. It removes the custom modulemap and allows CocoaPods to generate the umbrella header with grdb_config.h. I successfully tested both the standard and SQLCipher subspecs against these changes.

If you'd like I could also remove the use_frameworks! line from the README as it would be no longer required if this were merged.

Cheers!

@groue
Copy link
Owner

groue commented Jul 18, 2019

Hi @chrisballinger,

This looks like a much useful PR. I'll look at it when I'm back from vacations, in about a week. Thank you very much for the investigation job which is behind 👍

@chrisballinger
Copy link
Contributor Author

Thanks, no rush, enjoy your vacation!

Copy link
Owner

@groue groue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @chrisballinger,

Your PR works like a charm 💯

However, README#CocoaPods has not been updated. Please update it so that users know that both technique are supported.

Also, changes were not tested. Would you please allow me to push to your branch? I want to make sure both installation methods are tested on Travis (framework, and static). I have a commit ready, but I lack the permission to push. There should be a checkbox somewhere on the pull request page that allows you to do that (allow push from repository maintainers, something like that).

@groue groue added this to the GRDB 4.2.0 milestone Jul 28, 2019
@groue
Copy link
Owner

groue commented Jul 28, 2019

Suggestion:

CocoaPods is a dependency manager for Xcode projects. To use GRDB with CocoaPods (version 1.2 or higher), specify in your Podfile:

pod 'GRDB.swift'

GRDB can be installed as a framework, or a static library.

@groue
Copy link
Owner

groue commented Jul 28, 2019

Oh, and also please merge or rebase the latest development branch, and add your PR to the CHANGELOG: 570 goes right between 563 and 574. That'll nicely complete the PR :-)

@chrisballinger
Copy link
Contributor Author

@groue Thanks for taking a look! You should be able to push to that branch now. I'll try to put the finishing touches on this PR tomorrow.

@groue
Copy link
Owner

groue commented Jul 30, 2019

@groue Thanks for taking a look! You should be able to push to that branch now. I'll try to put the finishing touches on this PR tomorrow.

Push done! I hope Travis will swallow the new tests in the first stroke :-)

@chrisballinger chrisballinger force-pushed the fix-issue-360-modulemap branch from adb66e7 to d357f48 Compare July 30, 2019 23:28
@chrisballinger chrisballinger force-pushed the fix-issue-360-modulemap branch from 8a80c22 to bdf802a Compare July 30, 2019 23:34
@chrisballinger
Copy link
Contributor Author

@groue Rebased and added an entry to README and CHANGELOG

@groue groue merged commit e23cb4e into groue:development Jul 31, 2019
@groue
Copy link
Owner

groue commented Jul 31, 2019

Thank you @chrisballinger, welcome to the GRDB contributors!

I'd be happy to grant you push access to the repository. Drop me a line if you are interested!

groue added a commit that referenced this pull request Jul 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants