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

Bring cmake build up to parity with autotools and add documentation #24

Merged
merged 11 commits into from
Sep 27, 2016
Merged

Bring cmake build up to parity with autotools and add documentation #24

merged 11 commits into from
Sep 27, 2016

Conversation

jamiesnape
Copy link
Contributor

This refactors and enhances the cmake build to add missing targets for documentation and the test suite. It also adds the missing documentation for the cmake build, adds the necessary macros for exporting functions for building shared libraries on Windows using cmake, and adds exported targets and config files for ease of use of libccd in other cmake builds.

@jamiesnape
Copy link
Contributor Author

I added a CMake build to .travis.yml so there is CI coverage of this PR.

@jslee02
Copy link
Contributor

jslee02 commented Sep 26, 2016

I have a suggestion on configuring CMake config file and version file for libccd. CMake provides a helper function that can automatically generate those files. Using the function would be less buggy rather than maintaining the manual files manually.

@jamiesnape
Copy link
Contributor Author

Sure, if you prefer. I always deliberately avoid that function, but I can make the change.

@jslee02
Copy link
Contributor

jslee02 commented Sep 26, 2016

Oh, I thought it's the recommended way by CMake. If this is a matter of preference then I think it should be determined by @danfis but not me. Thanks for the consideration, though.

@mamoll
Copy link

mamoll commented Sep 26, 2016

👍 I'm not a libccd developer, but do use it in other cmake-based projects.

@danfis danfis merged commit 16b9379 into danfis:master Sep 27, 2016
@danfis
Copy link
Owner

danfis commented Sep 27, 2016

Merged, thanks guys.

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.

4 participants