[GitHub] [celix] PengZheng commented on a diff in pull request #409: Feature/rename zmq and jansson targets to align with conan

[GitBox <gi...@apache.org>]

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-unsubscribe@celix.apache.org

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