[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1124: MINIFICPP-1367 Add option to disable NanoFi build

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

fgerlits commented on a change in pull request #1124:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1124#discussion_r664728209



##########
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.")

Review comment:
       Is there a real reason why Python needs to depend on nanofi?  If not, can you create a Jira to remove this dependency, please?

##########
File path: CMakeLists.txt
##########
@@ -50,6 +50,7 @@ option(ENABLE_OPS "Enable Operations/zlib Tools" ON)
 option(ENABLE_JNI "Instructs the build system to enable the JNI extension" OFF)
 option(ENABLE_OPENCV "Instructs the build system to enable the OpenCV extension" OFF)
 option(ENABLE_OPC "Instructs the build system to enable the OPC extension" OFF)
+option(ENABLE_NANOFI "Instructs the build system to enable nanofi library" ON)

Review comment:
       Should the default be OFF?  I don't know how usable nanofi is right now, and we may want to remove it later.

##########
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:
       or we could continue with Greek letters, like it is done with hurricanes :)  α βιτ ΣΗαλλενγινγ το τΥπε, βυτ ιτ ιζ ποσσιβλε




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

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