martinzink commented on a change in pull request #1069:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1069#discussion_r629293827
##########
File path: CMakeLists.txt
##########
@@ -847,21 +847,17 @@ else()
include(DockerConfig)
endif()
-if(NOT WIN32)
# Create a custom build target that will run the linter.
get_property(extensions GLOBAL PROPERTY EXTENSION-LINTERS)
add_custom_target(linter
- COMMAND ${CMAKE_SOURCE_DIR}/thirdparty/google-styleguide/run_linter.sh
- ${CMAKE_SOURCE_DIR}/libminifi/include/ --
- ${CMAKE_SOURCE_DIR}/libminifi/src/
- COMMAND ${CMAKE_SOURCE_DIR}/thirdparty/google-styleguide/run_linter.sh
- ${CMAKE_SOURCE_DIR}/libminifi/include/ --
- ${CMAKE_SOURCE_DIR}/libminifi/test/
- COMMAND ${CMAKE_SOURCE_DIR}/thirdparty/google-styleguide/run_linter.sh
- ${CMAKE_SOURCE_DIR}/encrypt-config/ --
- ${CMAKE_SOURCE_DIR}/encrypt-config/
+ COMMAND python
${CMAKE_SOURCE_DIR}/thirdparty/google-styleguide/run_linter.py -i
Review comment:
Since the run_linter.py only checked the cpp files in the first argument
and only the h files in the second, we didnt check any h files in the
libminifi/test folder.
It also needlesly double checked the libminifi/include folder.
The cpplint.py doesnt require the filelist to be separated by header/source,
so its easier to just have one argument.
##########
File path: .github/workflows/ci.yml
##########
@@ -24,15 +24,20 @@ jobs:
brew update
brew install ossp-uuid boost flex [email protected] ccache sqliteodbc automake
autoconf
- id: setup_env
+ name: setup enviroment
run: |
echo
"PATH=/usr/lib/ccache:/usr/local/opt/ccache/bin:/usr/local/opt/ccache/libexec:$PATH"
>> $GITHUB_ENV
echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
sudo xcode-select -switch /Applications/Xcode_11.2.1.app
- - id: build
+ - name: build
run: |
export
PATH="/usr/local/opt/[email protected]/lib:/usr/local/opt/[email protected]/include:/usr/local/opt/[email protected]/bin:$PATH"
export PKG_CONFIG_PATH="/usr/local/opt/[email protected]/lib/pkgconfig"
- ./bootstrap.sh -e -t && cd build && cmake
-DCMAKE_BUILD_TYPE=Release -DENABLE_LUA_SCRIPTING=1 -DENABLE_AWS=ON
-DENABLE_AZURE=ON -DENABLE_SQL=ON -DCMAKE_VERBOSE_MAKEFILE=ON
-DCMAKE_RULE_MESSAGES=OFF -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. &&
cmake --build . --parallel 4 && make test ARGS="--timeout 300 -j4
--output-on-failure" && make linter
+ ./bootstrap.sh -e -t && cd build && cmake
-DCMAKE_BUILD_TYPE=Release -DENABLE_LUA_SCRIPTING=1 -DENABLE_AWS=ON
-DENABLE_AZURE=ON -DENABLE_SQL=ON -DCMAKE_VERBOSE_MAKEFILE=ON
-DCMAKE_RULE_MESSAGES=OFF -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. &&
cmake --build . --parallel 4
+ - name: test
Review comment:
separating the build/test/linter checks into steps, and naming them
accordingly makes the github ci output more readable
##########
File path: libminifi/test/KamikazeProcessor.h
##########
@@ -18,13 +18,15 @@
* limitations under the License.
*/
+#include <string>
Review comment:
Linter issues
previously libminifi/test/*.h files were ignored by the linter.
--
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]
