PragmaTwice commented on code in PR #1250:
URL: 
https://github.com/apache/incubator-kvrocks/pull/1250#discussion_r1098701344


##########
cmake/luajit.cmake:
##########
@@ -19,27 +19,42 @@ include_guard()
 
 include(cmake/utils.cmake)
 
-if((${CMAKE_SYSTEM_NAME} MATCHES "Darwin") AND (NOT 
CMAKE_OSX_DEPLOYMENT_TARGET))
+if ((${CMAKE_SYSTEM_NAME} MATCHES "Darwin") AND (NOT 
CMAKE_OSX_DEPLOYMENT_TARGET))
+  set(sw_cmd "sw_vers")
+  set(sw_arg "-productVersion")
+  execute_process(COMMAND ${sw_cmd} ${sw_arg}
+          WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
+          OUTPUT_VARIABLE OUTPUT
+          RESULT_VARIABLE STATUS
+          OUTPUT_STRIP_TRAILING_WHITESPACE)
+  if (STATUS EQUAL 0)
+    string(REGEX MATCH "[0-9]+.[0-9]+"
+            DEPLOYMENT_TARGET ${OUTPUT})
+    set(CMAKE_OSX_DEPLOYMENT_TARGET "${DEPLOYMENT_TARGET}")
+    message(STATUS "CMAKE_OSX_DEPLOYMENT_TARGET was set to: 
${DEPLOYMENT_TARGET}")
+  endif ()

Review Comment:
   CMake variables has no regular scope (in `if` or `include`), so I prefer 
more concrete names, e.g. `SW_CMD_OUTPUT` rather than `OUTPUT`, `SW_CMD_STATUS` 
rather than `STATUS`.
   
   Otherwise we can access this `OUTPUT` variable anywhere after the code above 
is executed.



-- 
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]

Reply via email to