tezc / sc

Common libraries and data structures for C.
BSD 3-Clause "New" or "Revised" License
2.26k stars 245 forks source link

Option to turn off tests in the build #42

Closed rafaeldelboni closed 3 years ago

rafaeldelboni commented 3 years ago

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

https://github.com/libcheck/check/issues/238#issuecomment-574258335 https://cmake.org/cmake/help/latest/module/FetchContent.html#examples

codecov[bot] commented 3 years ago

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 commented 3 years ago

Hi again :)

I have one question,

https://github.com/tezc/sc/blob/9db998bad4e7ae80b8d2d4d7e74d5bf90c4062cf/array/CMakeLists.txt#L16

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 commented 3 years ago

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

tezc commented 3 years ago

@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.