date icon indicating copy to clipboard operation
date copied to clipboard

Don't write CMAKE_CXX_STANDARD in cache

Open OlivierLDff opened this issue 3 years ago • 1 comments

Hi, currently, CMAKE_CXX_STANDARD is written in cache: https://github.com/HowardHinnant/date/blob/9ea5654c1206e19245dc21d8a2c433e090c8c3f5/CMakeLists.txt#L28

I don't think this is a good practice since it could modify standard in super build environment. It would be nice to use a standard per target as recommended by cmake.

if (NOT "${CMAKE_VERSION}" VERSION_LESS "3.8")
  target_compile_features(target_name PUBLIC cxx_std_17)
endif()

If user want to override the standard for date library (ie c++11 or something like that), then a cache variable DATE_CXX_STANDARD and TZ_CXX_STANDARD could be introduced.

It would result with something like that:

target_compile_features(date PUBLIC cxx_std_${DATE_CXX_STANDARD})

Would such a PR be considered?

You can see similar discussion in https://github.com/google/googletest/pull/2808 .

Have a nice day, and thanks for the incredible work.

OlivierLDff avatar Mar 28 '22 13:03 OlivierLDff

I think that requiring CMake 3.8 would be fine, as it was released ages ago. This would make it possible to avoid increasing the number of conditionals in CMakeLists.txt

Tachi107 avatar Aug 04 '22 16:08 Tachi107