-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Modifications suggested by conan team #662
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might update the conan_package/README too, now that it is linked in conan-center
conanbuildinfo.cmake | ||
!FindGLM.cmake |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need to explicitly include FindGLM? Does it match some other ignore pattern?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an exclusion of all .cmake files in the .gitignore of the top level directory:
https://github.com/g-truc/glm/blob/master/.gitignore
TARGET_COMPILE_DEFINITIONS(testGlm PUBLIC "${CONAN_DEFINES}") | ||
TARGET_LINK_LIBRARIES(testGlm PUBLIC "${CONAN_LIBS}") | ||
SET_TARGET_PROPERTIES(testGlm PROPERTIES LINK_FLAGS "${CONAN_EXE_LINKER_FLAGS}") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Header only doesnt need further config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback. I've also updated the readme: https://github.com/dimi309/glm/blob/master/util/conan-package/README.md
I don't see how the conan packaging configuration could possibly affect the build. It is located within its own directory and, since it requires no compilation, it doesn't even have a travis or appveyor setup.
I see travis tests broken, but master branch seems also broken, I think it is not because of this PR. |
The glm package has now been accepted to the conan-center repository:
https://bintray.com/conan/conan-center
At the same time, the conan team has suggested some modifications to the packaging configuration, which I am introducing with this pull request.