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

Option to turn off tests in the build #42

Merged
merged 5 commits into from
Feb 14, 2021
Merged

Option to turn off tests in the build #42

merged 5 commits into from
Feb 14, 2021

Conversation

rafaeldelboni
Copy link
Contributor

It's me again :)

I saw that your tests were running with my project test, I did some investigation and I saw that this is a common practice make a flag to be able to disable the tests externally

libcheck/check#238 (comment)
https://cmake.org/cmake/help/latest/module/FetchContent.html#examples

@codecov
Copy link

codecov bot commented Feb 14, 2021

Codecov Report

Merging #42 (c31aafd) into master (9db998b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #42   +/-   ##
=======================================
  Coverage   98.11%   98.11%           
=======================================
  Files          23       23           
  Lines        2231     2231           
  Branches      384      384           
=======================================
  Hits         2189     2189           
  Partials       42       42           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9db998b...c31aafd. Read the comment docs.

@tezc
Copy link
Owner

tezc commented Feb 14, 2021

Hi again :)

I have one question,

set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -Wall -Wextra -pedantic -Werror")

Do these options apply to .so file? Or not, because it's defined after add_library line? -g option may cause your so file include debugging info. If that's an issue for you, you can change these if checks with something like :

if (NOT CMAKE_C_COMPILER_ID MATCHES "MSVC")
    target_compile_options(${PROJECT_NAME}_test PRIVATE -g -Wall -Wextra -pedantic -Werror)
endif ()

Otherwise, feel free to merge this.

Thanks a lot.

@rafaeldelboni
Copy link
Contributor Author

Is not a issue for me :) I have no rights to merge :(

@tezc tezc merged commit dbfc704 into tezc:master Feb 14, 2021
@tezc
Copy link
Owner

tezc commented Feb 14, 2021

@rafaeldelboni

I couldn't find merge access option quickly, I'll take a look :)

Anyway, I've created v1.0.0 tag again, including this PR. Let me know everything works for you.

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.

None yet

2 participants