[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1029: MINIFICPP-1345 Add flake8 check for python files

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

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



##########
File path: .github/workflows/ci.yml
##########
@@ -156,10 +156,10 @@ jobs:
       - id: install_deps
         run: |
           sudo apt update
-          sudo apt install -y ccache libfl-dev libpcap-dev libboost-all-dev
+          sudo apt install -y ccache libfl-dev libpcap-dev libboost-all-dev flake8
           echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
       - id: build
-        run: ./bootstrap.sh -e -t && cd build  && cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_BUSTACHE=ON -DENABLE_SQLITE=ON -DENABLE_PCAP=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && make -j4 VERBOSE=1  && make test ARGS="--timeout 300 -j2 --output-on-failure"
+        run: ./bootstrap.sh -e -t && cd build  && cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_BUSTACHE=ON -DENABLE_SQLITE=ON -DENABLE_PCAP=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && make -j4 VERBOSE=1  && make test ARGS="--timeout 300 -j2 --output-on-failure" && make flake8

Review comment:
       I would make this a separate step, as it's not really part of the build.  That would also avoid a conflict with #1026.

##########
File path: run_flake8.sh
##########
@@ -0,0 +1,6 @@
+#!/bin/bash
+
+set -euo pipefail
+
+directory=${1:-.}
+flake8 --exclude thirdparty,build --ignore E501,W503 --per-file-ignores="steps.py:F811" "${directory}"

Review comment:
       If we add `--builtins log,REL_SUCCESS,REL_FAILURE` here, then we can get rid of most of the `# noqa: F821` comments.




-- 
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:
users@infra.apache.org