absurdfarce commented on code in PR #582:
URL:
https://github.com/apache/cassandra-cpp-driver/pull/582#discussion_r2991463648
##########
CMakeLists.txt:
##########
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.10)
+cmake_minimum_required(VERSION 3.5)
Review Comment:
Can you elaborate on a couple things here?
First: why is it necessary to change the cmake minimum version at all in
order to fix this?
Second: if it _is_ necessary to change it why are we tweaking it only in
three CMakeLists files rather than uniformly throughout the project?
##########
CMakeLists.txt:
##########
@@ -177,7 +178,8 @@ if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR
endif()
endif()
- if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
+ if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR
+ "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang")
Review Comment:
I'll let him speak on this for sure but I believe @yifan-c was able to get
this working more generally using regex matching:
```
if("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang$"
```
That's probably more desirable as it covers a broader range of clang
derivatives. cmake supports a number of such variants so as long as the flags
in question apply to clang generally (i.e. isn't some custom flag implemented
for a specific clang variant) the more general syntax seems preferrable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
