boostorg / boostorg/gil

Move operations only relevant for test out of main CMakeLists.txt

Open
#168 6 comments 0 reactions 1 assignee Claimed by @mloskot View on GitHub
config/cmake status/need-feedback status/work-in-progress
Dominant language
C++
Stars
199
Forks
171
Avg merge
1d 14h
Merged PRs (30d)
10

Description

Just a comment based on my person preference and experience:

I'd recommend moving all the compiler-flags and other machinery that are not necessary for using the library itself into the respective CMakeLists.txt files where they are actually needed.

This is more relevant, when you provide a cmake target (see https://github.com/boostorg/gil/issues/167) and/or have to compile your library, but even if not, I generally prefer to put compiler options next to the targets they are actually relevant for. E.g. `Boost::unit_test_framework` is afaik only needed when you want to run the unit-tests. So it imho should only be looked up when `gil/test/CMakeLists.txt` gets actually included (e.g. not if `GIL_BUILD_TESTS == OFF`). Same with things like `/Wall` or `CMAKE_CXX_STANDARD 11`.

Even if you are not defining a cmake library target (for which you should definetifely not set public warning level flags) it makes it easier for me to discover, which flags I have to set in my project when I want to use this library, vs what flags you are using in order to find bugs in the implementation.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.