[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1029: MINIFICPP-1345 Add flake8 check for python files
lordgamez commented on a change in pull request #1029: URL: https://github.com/apache/nifi-minifi-cpp/pull/1029#discussion_r604017808 ## 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 agree, updated in 51b7f780e0497c5695321631e9c7bd0f655ddcc1 -- 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: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1029: MINIFICPP-1345 Add flake8 check for python files
lordgamez commented on a change in pull request #1029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1029#discussion_r604017492
##
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:
Good idea! Updated in 51b7f780e0497c5695321631e9c7bd0f655ddcc1
--
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:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1029: MINIFICPP-1345 Add flake8 check for python files
lordgamez commented on a change in pull request #1029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1029#discussion_r596250644
##
File path: docker/test/integration/minifi/processors/DeleteS3Object.py
##
@@ -1,13 +1,16 @@
from ..core.Processor import Processor
+
class DeleteS3Object(Processor):
-def __init__(self,
-proxy_host = '',
-proxy_port = '',
-proxy_username = '',
-proxy_password = ''):
-super(DeleteS3Object, self).__init__('DeleteS3Object',
-properties = {
+def __init__(
+self,
Review comment:
The [pep8
recommendation](https://www.python.org/dev/peps/pep-0008/#indentation) is the
same in this case:
```
# Add 4 spaces (an extra level of indentation) to distinguish arguments from
the rest.
def long_function_name(
var_one, var_two, var_three,
var_four):
print(var_one)
```
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:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1029: MINIFICPP-1345 Add flake8 check for python files
lordgamez commented on a change in pull request #1029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1029#discussion_r596248851
##
File path: docker/test/integration/minifi/processors/DeleteS3Object.py
##
@@ -1,13 +1,16 @@
from ..core.Processor import Processor
+
class DeleteS3Object(Processor):
-def __init__(self,
-proxy_host = '',
-proxy_port = '',
-proxy_username = '',
-proxy_password = ''):
-super(DeleteS3Object, self).__init__('DeleteS3Object',
-properties = {
+def __init__(
+self,
Review comment:
In that case we would have 2 problems with flake8:
./docker/test/integration/minifi/processors/DeleteS3Object.py:10:5: E125
continuation line with same indent as next logical line
./docker/test/integration/minifi/processors/DeleteS3Object.py:11:13: E117
over-indented
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:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1029: MINIFICPP-1345 Add flake8 check for python files
lordgamez commented on a change in pull request #1029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1029#discussion_r596241246
##
File path: docker/test/integration/minifi/processors/DeleteS3Object.py
##
@@ -1,13 +1,16 @@
from ..core.Processor import Processor
+
class DeleteS3Object(Processor):
-def __init__(self,
-proxy_host = '',
-proxy_port = '',
-proxy_username = '',
-proxy_password = ''):
-super(DeleteS3Object, self).__init__('DeleteS3Object',
-properties = {
+def __init__(
+self,
Review comment:
Having one level of indentation here would have the same level
indentation as the following block, which is forbidden by
[E125](https://www.flake8rules.com/rules/E125.html). The recommendation is to
have another level indentation here.
##
File path: docker/test/integration/minifi/processors/DeleteS3Object.py
##
@@ -1,13 +1,16 @@
from ..core.Processor import Processor
+
class DeleteS3Object(Processor):
-def __init__(self,
-proxy_host = '',
-proxy_port = '',
-proxy_username = '',
-proxy_password = ''):
-super(DeleteS3Object, self).__init__('DeleteS3Object',
-properties = {
+def __init__(
+self,
Review comment:
Having one level of indentation here would have the same level of
indentation as the following block, which is forbidden by
[E125](https://www.flake8rules.com/rules/E125.html). The recommendation is to
have another level indentation here.
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:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1029: MINIFICPP-1345 Add flake8 check for python files
lordgamez commented on a change in pull request #1029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1029#discussion_r595004676
##
File path: run_flake8.sh
##
@@ -0,0 +1,6 @@
+#!/bin/bash
+
+set -euo pipefail
+
+directory=${1:-.}
+flake8 --exclude thirdparty,build --ignore E501,W504
--per-file-ignores="steps.py:F811" "${directory}"
Review comment:
Yes, in hindsight it makes more sense to go with that, even the [flake8
rule](https://www.flake8rules.com/rules/W503.html) mentions that this goes
against the pep8 recommendation. I added the change in
f417b932e4f76b74351c78739eefd1d462dac4b4
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:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1029: MINIFICPP-1345 Add flake8 check for python files
lordgamez commented on a change in pull request #1029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1029#discussion_r594988935
##
File path: run_flake8.sh
##
@@ -0,0 +1,6 @@
+#!/bin/bash
+
+set -euo pipefail
+
+directory=${1:-.}
+flake8 --exclude thirdparty,build --ignore E501,W504
--per-file-ignores="steps.py:F811" "${directory}"
Review comment:
Flake8 has both [W503](https://www.flake8rules.com/rules/W503.html) and
[W504](https://www.flake8rules.com/rules/W504.html) enabled by default and they
contradict each other. We have to choose one of them to use, in this case I
chose to use W503 and ignore W504 to have the binary operators at the end of
the line.
[F811](https://www.flake8rules.com/rules/F811.html) is only ignored in
steps.py as it defines the `behave` framework steps. This file has the same
`step_impl` function name for each step and `flake8` detects it as a
redefinition.
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:
us...@infra.apache.org
