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
don't build/run meta cpp examples in interface travis builds #4155
don't build/run meta cpp examples in interface travis builds #4155
Conversation
@@ -25,42 +25,42 @@ matrix: | |||
- compiler: clang | |||
services: docker | |||
env: | |||
- CMAKE_OPTIONS="-DINTERFACE_PYTHON=ON -DTRAVIS_DISABLE_UNIT_TESTS=ON -DTRAVIS_DISABLE_LIBSHOGUN_TESTS=ON" | |||
- CMAKE_OPTIONS="-DINTERFACE_PYTHON=ON -DTRAVIS_DISABLE_UNIT_TESTS=ON -DTRAVIS_DISABLE_LIBSHOGUN_TESTS=ON -DTRAVIS_DISABLE_META_CPP=ON" |
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.
before merging, and since you are touching this could you move all these common parameters into a variable instead of keep growing this and copy-pasting around. thnx
DTRAVIS_DISABLE_UNIT_TESTS=ON -DTRAVIS_DISABLE_LIBSHOGUN_TESTS=ON -DTRAVIS_DISABLE_META_CPP=ON
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.
btw should the libshogun/travis examples be actually part of make all
at all?
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.
no opinion on the second. I just think building and executing them in these interface builds is a waste of cpu
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.
maybe removing meta examples and libshogun all together from make all
would make more sense and then just call the 'right' make for the right meta example you would like to have in the test phase, as that's actually more related to test and not the library testing itself.
https://cmake.org/cmake/help/v3.5/prop_dir/EXCLUDE_FROM_ALL.html
and
https://cmake.org/cmake/help/v3.0/prop_tgt/EXCLUDE_FROM_DEFAULT_BUILD.html
bacause of msvc
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.
and maybe examples should have a common target... say make examples
that actually collects all the required examples. that you would like to see with the given INTERFACE combination...
this is in context of trying to drop these TRAVIS_DISABLE_*
params as they are rather hack from the past than solutions
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.
+1 for all of this, but I think that would be another PR
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.
another? i mean at least a var would be nice.... these are quite easy fixes in cmake files though
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.
no agreed, var should be this one, I meant the other stuff
No description provided.