-
Notifications
You must be signed in to change notification settings - Fork 26
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 cmake_common_scripts #40
Add cmake_common_scripts #40
Conversation
a0cc773
to
420f098
Compare
7564899
to
44fcae3
Compare
44fcae3
to
3f04047
Compare
6357b5e
to
68255e4
Compare
@mpowelson Ready for review. |
if(NOT EIGEN3_INCLUDE_DIRS) | ||
set(EIGEN3_INCLUDE_DIRS ${EIGEN3_INCLUDE_DIR}) | ||
endif() |
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.
What is this? You're allowed to overwrite the version of Eigen found by 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.
I believe this is just making sure the variable EIGEN3_INCLUDE_DIRS exists and if not it sets it to EIGEN3_INCLUDE_DIR. They should be the same but I believe older versions may have only had EIGEN3_INCLUDE_DIR which was an error in Eigen.
@mpowelson I fixed the requested changes. Also I create tag v0.1.0 and increased the version to 0.2.0 since this adds a news dependency. |
Ok. It looks good to me. |
No description provided.