jiridanek commented on a change in pull request #319:
URL: https://github.com/apache/qpid-proton/pull/319#discussion_r648031232
##########
File path: cpp/CMakeLists.txt
##########
@@ -36,6 +36,14 @@ else()
set(CONNECT_CONFIG_SRC src/connect_config_dummy.cpp)
endif()
+# Check for OPENTELEMETRY-CPP for distributed tracing
+find_package(opentelemetry-cpp)
+cmake_dependent_option(ENABLE_OPENTELEMETRYCPP "Use opentelemetry for
distributed tracing" ON "OPENTELEMETRY-CPP_FOUND" OFF)
+
+if (ENABLE_OPENTELEMETRYCPP)
+ include_directories(${OPENTELEMETRY_CPP_INCLUDE_DIRS})
Review comment:
This is "old-style" CMake. Nowadays, you should use
`target_link_libraries`
(https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#transitive-usage-requirements)
later when you compile something that needs OpenTelemetry. Adding an
OpenTelemetry-exported target this way will also add the include directories to
the compilation.
If you need to adjust includes yourself (which should not be necessary
here), you should use `target_include_directories`
(https://cmake.org/cmake/help/v2.8.12/cmake.html#command:target_include_directories).
These things are available even in CMake 2.8.12 which we need to support in
Proton.
# Docs
Here are some documents that introduce "Modern CMake". If these things can
be used in CMake 2.8.12, then they should be used.
* https://cliutils.gitlab.io/modern-cmake/
* https://gist.github.com/mbinna/c61dbb39bca0e4fb7d1f73b0d66a4fd1
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
