search

HowardHinnant / date

A date and time library based on the C++11/14/17 <chrono> header
Other
3.14k stars 677 forks source link

Don't write CMAKE_CXX_STANDARD in cache #729

Open OlivierLDff opened 2 years ago

OlivierLDff commented 2 years ago

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.

Tachi107 commented 2 years ago

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