PengZheng commented on code in PR #409: URL: https://github.com/apache/celix/pull/409#discussion_r842348371
########## CMakeLists.txt: ########## @@ -32,7 +32,7 @@ find_package(ZLIB REQUIRED) #framework find_package(UUID REQUIRED) #framework Review Comment: Following the same logic, we should also update find modules for uuid, libzip, and ffi? ``` ########## CMakeLists.txt: ########## @@ -32,7 +32,7 @@ find_package(ZLIB REQUIRED) #framework find_package(UUID REQUIRED) #framework find_package(CURL REQUIRED) #framework, etcdlib find_package(LIBZIP REQUIRED) #framework, etcdlib -find_package(Jansson REQUIRED) #etcdlib, dfi +find_package(jansson REQUIRED) #etcdlib, dfi Review Comment: nitpick: message("Note jansson::jansson target not created by find_package(Jansson). Creating jansson::jansson Target.") should also be updated. ########## cmake/Modules/Findczmq.cmake: ########## @@ -37,14 +37,14 @@ set(CZMQ_INCLUDE_DIRS ${CZMQ_INCLUDE_DIR} ) include(FindPackageHandleStandardArgs) # handle the QUIETLY and REQUIRED arguments and set CZMQ_FOUND to TRUE # if all listed variables are TRUE -find_package_handle_standard_args(CZMQ DEFAULT_MSG +find_package_handle_standard_args(czmq DEFAULT_MSG CZMQ_LIBRARY CZMQ_INCLUDE_DIR) mark_as_advanced(CZMQ_INCLUDE_DIR CZMQ_LIBRARY) -if (CZMQ_FOUND AND NOT TARGET CZMQ::lib) - add_library(CZMQ::lib SHARED IMPORTED) - set_target_properties(CZMQ::lib PROPERTIES +if (CZMQ_FOUND AND NOT TARGET czmq::czmq) + add_library(czmq::czmq SHARED IMPORTED) Review Comment: I noticed that test_cxx_remote_services_integration is still using old targets. ########## cmake/Modules/FindZeroMQ.cmake: ########## @@ -18,34 +18,34 @@ # - Try to find ZMQ # Once done this will define -# ZMQ_FOUND - System has Zmq -# ZMQ_INCLUDE_DIRS - The Zmq include directories -# ZMQ_LIBRARIES - The libraries needed to use Zmq -# ZMQ_DEFINITIONS - Compiler switches required for using Zmq -# ZMQ::lib - Imported target for UUID +# ZEROMQ_FOUND - System has Zmq +# ZEROMQ_INCLUDE_DIRS - The Zmq include directories +# ZEROMQ_LIBRARIES - The libraries needed to use Zmq +# ZEROMQ_DEFINITIONS - Compiler switches required for using Zmq +# ZeroMQ::ZeroMQ - Imported target for UUID -find_path(ZMQ_INCLUDE_DIR zmq.h zmq_utils.h +find_path(ZEROMQ_INCLUDE_DIR zmq.h zmq_utils.h /usr/include /usr/local/include ) -find_library(ZMQ_LIBRARY NAMES zmq +find_library(ZEROMQ_LIBRARY NAMES zmq PATHS /usr/lib /usr/local/lib /usr/lib64 /usr/local/lib64 ) -set(ZMQ_LIBRARIES ${ZMQ_LIBRARY} ) -set(ZMQ_INCLUDE_DIRS ${ZMQ_INCLUDE_DIR} ) +set(ZEROMQ_LIBRARIES ${ZEROMQ_LIBRARY} ) +set(ZEROMQ_INCLUDE_DIRS ${ZEROMQ_INCLUDE_DIR} ) include(FindPackageHandleStandardArgs) # handle the QUIETLY and REQUIRED arguments and set ZMQ_FOUND to TRUE Review Comment: nitpick: ZEROMQ_FOUND -- 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: dev-unsubscr...@celix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org