szaszm commented on a change in pull request #1124:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1124#discussion_r664080797
##########
File path: CMakeLists.txt
##########
@@ -589,12 +592,12 @@ if (NOT DISABLE_CURL AND NOT DISABLE_CONTROLLER)
endif()
-if (NOT DISABLE_CURL)
- if (ENABLE_PYTHON)
- if (NOT WIN32)
- add_subdirectory(python/library)
- endif()
- endif(ENABLE_PYTHON)
+if (NOT DISABLE_CURL AND ENABLE_PYTHON AND NOT WIN32)
+ if (ENABLE_NANOFI)
+ add_subdirectory(python/library)
+ else()
+ message(FATAL_ERROR "Nanofi, a dependency of the python
extension is disabled, therefore Python extension cannot be enabled.")
+ endif()
Review comment:
We have some strange indentation here.
##########
File path: cmake/BuildTests.cmake
##########
@@ -39,51 +39,54 @@ if(NOT EXCLUDE_BOOST)
endif()
function(appendIncludes testName)
- target_include_directories(${testName} SYSTEM BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/thirdparty/catch")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/include")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/c2/protocols")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/c2")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/core")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/core/controller")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/core/repository")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/core/yaml")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/core/statemanagement")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/core/statemanagement/metrics")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/io")
- if(WIN32)
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/opsys/win")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/opsys/win/io")
- else()
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/opsys/posix")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/opsys/posix/io")
- endif()
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/utils")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/processors")
- target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/provenance")
+ target_include_directories(${testName} SYSTEM BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/thirdparty/catch")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/include")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/c2/protocols")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/c2")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/core")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/core/controller")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/core/repository")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/core/yaml")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/core/statemanagement")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/core/statemanagement/metrics")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/io")
+ if(WIN32)
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/opsys/win")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/opsys/win/io")
+ else()
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/opsys/posix")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/opsys/posix/io")
+ endif()
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/utils")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/processors")
+ target_include_directories(${testName} BEFORE PRIVATE
"${CMAKE_SOURCE_DIR}/libminifi/include/provenance")
endfunction()
function(createTests testName)
- message ("-- Adding test: ${testName}")
- appendIncludes("${testName}")
+ message ("-- Adding test: ${testName}")
Review comment:
Unrelated, but we should really change this to `message(DEBUG, "...")`.
It would be nice to reduce the build output to useful info only.
##########
File path: cmake/BuildTests.cmake
##########
@@ -120,29 +123,30 @@ FOREACH(testfile ${UNIT_TESTS})
ENDFOREACH()
message("-- Finished building ${UNIT_TEST_COUNT} unit test file(s)...")
-if(NOT WIN32)
+if(NOT WIN32 AND ENABLE_NANOFI)
Review comment:
Can you increase the indentation of the contents, until line 147?
##########
File path: bstrp_functions.sh
##########
@@ -368,6 +390,7 @@ show_supported_features() {
echo "W. Openwsman Support ...........$(print_feature_status
OPENWSMAN_ENABLED)"
echo "X. Azure Support ...............$(print_feature_status AZURE_ENABLED)"
echo "Y. Systemd Support .............$(print_feature_status
SYSTEMD_ENABLED)"
+ echo "Z. NanoFi Support ..............$(print_feature_status NANOFI_ENABLED)"
Review comment:
We ran out of letters. I wonder what's going to happen to the next
feature.
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
