You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "assignUser (via GitHub)" <gi...@apache.org> on 2023/04/06 13:26:05 UTC

[GitHub] [arrow] assignUser commented on a diff in pull request #34920: GH-34813: [C++] Improve GoogleTest detection

assignUser commented on code in PR #34920:
URL: https://github.com/apache/arrow/pull/34920#discussion_r1159797944


##########
cpp/CMakeLists.txt:
##########
@@ -839,8 +839,8 @@ if(NOT MSVC_TOOLCHAIN)
   list(APPEND ARROW_SHARED_LINK_LIBS ${CMAKE_DL_LIBS})
 endif()
 
-set(ARROW_TEST_LINK_TOOLCHAIN arrow::flatbuffers GTest::gtest_main GTest::gtest
-                              GTest::gmock)
+set(ARROW_TEST_LINK_TOOLCHAIN arrow::flatbuffers ${ARROW_GTEST_GTEST_MAIN}

Review Comment:
   IMHO this is a regression as using variables instead of targets is an anti-pattern but I currently don't see a cleaner approach either. 
   
   I think we should rethink the entire dependency management once we have bumped the cmake version to 3.16 as we will have much better tools available. (this is of course out of scope for this PR)



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org