assignUser commented on code in PR #341:
URL: https://github.com/apache/arrow-nanoarrow/pull/341#discussion_r1433356766


##########
extensions/nanoarrow_ipc/CMakeLists.txt:
##########
@@ -215,6 +215,16 @@ if(NANOARROW_IPC_BUILD_TESTS)
     fetchcontent_makeavailable(nlohmann_json)
   endif()
 
+  # zlib to decode gzipped integration testing JSON files
+  # We don't use Arrow C++ for this because building Arrow C++ with zlib
+  # is not trivial on Windows.
+  find_package(ZLIB)
+  if(NOT ZLIB_FOUND)
+    # Wrapper around FetchContent that better isolates the zlib CMakeLists.txt

Review Comment:
   Yeah, separate dir scope can really help prevent global options from being 
modified ^^



##########
dev/release/verify-release-candidate.sh:
##########
@@ -217,6 +217,7 @@ test_cmake_project() {
   show_info "Configure CMake Project"
   ${CMAKE_BIN} "${NANOARROW_SOURCE_DIR}/${2}" \
     "${@:3}" \
+    -DCMAKE_POSITION_INDEPENDENT_CODE=ON \

Review Comment:
   This works of course but out of interest what was the issue with the target 
version? It reads like you set the flags directly but you can also apply this 
option on a target basis and let cmake add the flags: `set_property(TARGET 
fpic-test PROPERTY POSITION_INDEPENDENT_CODE ON)`



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