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

Add CMake knobs to suppress building tests #287

Merged
merged 2 commits into from
Nov 24, 2016
Merged

Conversation

ismaell
Copy link

@ismaell ismaell commented Nov 23, 2016

  • BUILD_NTOX for nTox
  • BUILD_AV_TEST for the toxav test

This change is Reviewable

@iphydf
Copy link
Member

iphydf commented Nov 23, 2016

Thanks for your contribution, this is useful to have. I just have one minor remark. Let me know if you feel otherwise.


Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


CMakeLists.txt, line 468 at r1 (raw file):

option(BUILD_AV_TEST "Build toxav test" ON)
if(NOT WIN32 AND BUILD_AV_TEST
   AND BUILD_TOXAV AND SNDFILE_FOUND AND PORTAUDIO_FOUND AND OPENCV_FOUND)

Minor nit: put the BUILD_* on one line, and the *_FOUND ones on the next line.


Comments from Reviewable

@iphydf
Copy link
Member

iphydf commented Nov 23, 2016

Ok, thanks. This looks fine. Can you make it either 1 or 2 commits? The reordering doesn't need to be its own commit.

@iphydf
Copy link
Member

iphydf commented Nov 23, 2016

:lgtm:


Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@iphydf iphydf merged commit 19711d0 into TokTok:master Nov 24, 2016
@iphydf iphydf added this to the v0.0.5 milestone Nov 24, 2016
@GrayHatter
Copy link

@ismaell thanks mate! I've been commenting them out otherwise... this is already really helpful!!!

@ismaell
Copy link
Author

ismaell commented Nov 24, 2016

@GrayHatter you're welcome.

This pull request was closed.
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.

3 participants