[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1247: MINIFICPP-1731 - Linter targets should not be built by default on Win…
lordgamez commented on a change in pull request #1247:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1247#discussion_r789512942
##
File path: CMakeLists.txt
##
@@ -872,25 +872,27 @@ include(BuildDocs)
# Create a custom build target that will run the linter.
# Directories have their separate linter targets to be able to use better
parallelization
-get_property(extensions GLOBAL PROPERTY EXTENSION-LINTERS)
-set(root_linted_dirs libminifi/include libminifi/src libminifi/test
encrypt-config)
-list(TRANSFORM root_linted_dirs PREPEND ${CMAKE_SOURCE_DIR}/)
-
-set(linted_dir_counter 1)
-set(root_linter_target_names "")
-
-foreach(linted_dir ${root_linted_dirs})
- set(linter_target_name "root-linter-${linted_dir_counter}")
- list(APPEND root_linter_target_names ${linter_target_name})
- add_custom_target("${linter_target_name}"
- COMMAND python3
${CMAKE_SOURCE_DIR}/thirdparty/google-styleguide/run_linter.py -q -i
- ${linted_dir}
- )
- math(EXPR linted_dir_counter "${linted_dir_counter}+1")
-endforeach()
-
-# Main linter target that depends on every other
-add_custom_target(linter DEPENDS ${root_linter_target_names} ${extensions})
+if (ENABLE_LINTER)
+get_property(extensions GLOBAL PROPERTY EXTENSION-LINTERS)
Review comment:
This cmake file is indented with tabs so the new content should also use
tabs instead of spaces
##
File path: CMakeLists.txt
##
@@ -872,25 +872,27 @@ include(BuildDocs)
# Create a custom build target that will run the linter.
# Directories have their separate linter targets to be able to use better
parallelization
-get_property(extensions GLOBAL PROPERTY EXTENSION-LINTERS)
-set(root_linted_dirs libminifi/include libminifi/src libminifi/test
encrypt-config)
-list(TRANSFORM root_linted_dirs PREPEND ${CMAKE_SOURCE_DIR}/)
-
-set(linted_dir_counter 1)
-set(root_linter_target_names "")
-
-foreach(linted_dir ${root_linted_dirs})
- set(linter_target_name "root-linter-${linted_dir_counter}")
- list(APPEND root_linter_target_names ${linter_target_name})
- add_custom_target("${linter_target_name}"
- COMMAND python3
${CMAKE_SOURCE_DIR}/thirdparty/google-styleguide/run_linter.py -q -i
- ${linted_dir}
- )
- math(EXPR linted_dir_counter "${linted_dir_counter}+1")
-endforeach()
-
-# Main linter target that depends on every other
-add_custom_target(linter DEPENDS ${root_linter_target_names} ${extensions})
+if (ENABLE_LINTER)
Review comment:
Could you please explain how does this change the behavior on Windows?
It looks like the ENABLE_LINTER is still ON by default and we did not change
anything that is specific to Windows.
--
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
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1247: MINIFICPP-1731 - Linter targets should not be built by default on Win…
lordgamez commented on a change in pull request #1247:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1247#discussion_r789532760
##
File path: CMakeLists.txt
##
@@ -872,25 +872,27 @@ include(BuildDocs)
# Create a custom build target that will run the linter.
# Directories have their separate linter targets to be able to use better
parallelization
-get_property(extensions GLOBAL PROPERTY EXTENSION-LINTERS)
-set(root_linted_dirs libminifi/include libminifi/src libminifi/test
encrypt-config)
-list(TRANSFORM root_linted_dirs PREPEND ${CMAKE_SOURCE_DIR}/)
-
-set(linted_dir_counter 1)
-set(root_linter_target_names "")
-
-foreach(linted_dir ${root_linted_dirs})
- set(linter_target_name "root-linter-${linted_dir_counter}")
- list(APPEND root_linter_target_names ${linter_target_name})
- add_custom_target("${linter_target_name}"
- COMMAND python3
${CMAKE_SOURCE_DIR}/thirdparty/google-styleguide/run_linter.py -q -i
- ${linted_dir}
- )
- math(EXPR linted_dir_counter "${linted_dir_counter}+1")
-endforeach()
-
-# Main linter target that depends on every other
-add_custom_target(linter DEPENDS ${root_linter_target_names} ${extensions})
+if (ENABLE_LINTER)
Review comment:
Oh now I see, so it's more about the creation of the target than about
building the target. It's clear now, thanks!
--
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
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1247: MINIFICPP-1731 - Linter targets should not be built by default on Win…
lordgamez commented on a change in pull request #1247:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1247#discussion_r789540945
##
File path: CMakeLists.txt
##
@@ -872,25 +872,27 @@ include(BuildDocs)
# Create a custom build target that will run the linter.
# Directories have their separate linter targets to be able to use better
parallelization
-get_property(extensions GLOBAL PROPERTY EXTENSION-LINTERS)
-set(root_linted_dirs libminifi/include libminifi/src libminifi/test
encrypt-config)
-list(TRANSFORM root_linted_dirs PREPEND ${CMAKE_SOURCE_DIR}/)
-
-set(linted_dir_counter 1)
-set(root_linter_target_names "")
-
-foreach(linted_dir ${root_linted_dirs})
- set(linter_target_name "root-linter-${linted_dir_counter}")
- list(APPEND root_linter_target_names ${linter_target_name})
- add_custom_target("${linter_target_name}"
- COMMAND python3
${CMAKE_SOURCE_DIR}/thirdparty/google-styleguide/run_linter.py -q -i
- ${linted_dir}
- )
- math(EXPR linted_dir_counter "${linted_dir_counter}+1")
-endforeach()
-
-# Main linter target that depends on every other
-add_custom_target(linter DEPENDS ${root_linter_target_names} ${extensions})
+if (ENABLE_LINTER)
+get_property(extensions GLOBAL PROPERTY EXTENSION-LINTERS)
Review comment:
Actually as I checked although as the file already has 15 other lines
where spaces are used for indentation it is not that important.
--
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
