szaszm commented on code in PR #2155:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2155#discussion_r3053048119
##########
extensions/sql/CMakeLists.txt:
##########
@@ -23,15 +23,22 @@ endif()
include(${CMAKE_SOURCE_DIR}/extensions/ExtensionHeader.txt)
-include_directories(SYSTEM ../../thirdparty/rapidjson-1.1.0/include/
../../thirdparty/rapidjson-1.1.0/include/rapidjson)
-include_directories(".")
-
file(GLOB SOURCES "*.cpp" "services/*.cpp" "processors/*.cpp" "data/*.cpp")
add_minifi_library(minifi-sql SHARED ${SOURCES})
-if(WIN32)
+# Modern CMake: Target-specific includes instead of global
include_directories()
Review Comment:
I don't think this needs a comment, it's expected knowledge of people
working with cmake nowadays that legacy global and directory-based commands are
to be avoided.
##########
extensions/sql/CMakeLists.txt:
##########
@@ -23,15 +23,22 @@ endif()
include(${CMAKE_SOURCE_DIR}/extensions/ExtensionHeader.txt)
-include_directories(SYSTEM ../../thirdparty/rapidjson-1.1.0/include/
../../thirdparty/rapidjson-1.1.0/include/rapidjson)
-include_directories(".")
-
file(GLOB SOURCES "*.cpp" "services/*.cpp" "processors/*.cpp" "data/*.cpp")
add_minifi_library(minifi-sql SHARED ${SOURCES})
-if(WIN32)
+# Modern CMake: Target-specific includes instead of global
include_directories()
+target_include_directories(minifi-sql
+ PRIVATE
+ "."
+ SYSTEM PRIVATE
+ "../../thirdparty/rapidjson-1.1.0/include/"
+ "../../thirdparty/rapidjson-1.1.0/include/rapidjson"
Review Comment:
instead of adding the dependency include dirs explicitly, the proper
approach would be using target_link_libraries against the rapidjson target
##########
core-framework/common/include/utils/expected.h:
##########
Review Comment:
the monadic operations have been standardized, so our implementations can be
removed
--
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]
