Public vs Private warnings

[huand]

Hello,

Please note that im no CMake expert. Should this line really be with PUBLIC property? why not PRIVATE?

ModernCppStarter/test/CMakeLists.txt

Line 37 in b347492

target_compile_options(Greeter PUBLIC -Wall -Wpedantic -Wextra -Werror)

[ClausKlein]

IMHO: for a lib it would be better is is PRIVATE ...

if it is like here, the user inherit the same compile options, this may breack its build

[TheLartians]

These flags are only set when configuring the tests CMakeLists (either through cmake -Stest ... or cmake -Sall ...). The reason that they are set to PUBLIC is to ensure that the tests targets themselves also inherit them, for enabling the warnings in tests and also to be able to check single headers for warnings as they would only appear in the derived targets.

Note that regular users of the library will only consume the main top-level CMakeLists and therefore won't activate or inherit the warnings.

[huand]

Ok, but it is a bit counter-intuitive that what prevents compilation in standalone is something set in test.
I understand that users that want to write some standalone should not use the all/CMakeLists.

If I clone your project and just have an empty main in standalone I try to compile with your instructions

cmake -S all -B build
cmake --build build

I get

/home/hubert/Downloads/ModernCppStarter-master/standalone/source/main.cpp:9:15: error: unused parameter ‘argc’ [-Werror=unused-parameter]
    9 | auto main(int argc, char** argv) -> int { return 0; }
      |                     ~~~~~~~^~~~
cc1plus: all warnings being treated as errors
gmake[2]: *** [standalone/CMakeFiles/GreeterStandalone.dir/build.make:82: standalone/CMakeFiles/GreeterStandalone.dir/source/main.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:321: standalone/CMakeFiles/GreeterStandalone.dir/all] Error 2
gmake: *** [Makefile:160: all] Error 2

but if i put the PRIVATE keyword for warnings then it compiles fine

[ 70%] Built target GreeterStandalone

[huand]

is this the intended behaviour?

[TheLartians]

That’s a good point, I initially thought it may be a good idea to check any other purposefully included targets for warnings when tests are included, but I agree that it is unexpected behavior.
It might therefore be a better idea to make the flags private and only apply them to the greeter and test targets after all.

Feel free to open a PR if you want!