[v1] maint: Use flake8 to check python code

[libvirt] [PATCH] maint: Use flake8 to check python code

Posted by Shi Lei 4 years, 7 months ago

Replace 'sc_prohibit_semicolon_at_eol_in_python' with generic 'sc_flake8' rule
to check python code style.

Now 'sc_flake8' just check the error E703: 'statement ends with a semicolon'.
In future, we could use '--select' to introduce more rules.

Signed-off-by: Shi Lei <shi_lei@massclouds.com>
---
 cfg.mk       | 8 +++-----
 configure.ac | 4 ++++
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 42e1abf0..8acc45ac 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -812,11 +812,9 @@ sc_require_enum_last_marker:
 	    exit 1; } || :
 
 # In Python files we don't want to end lines with a semicolon like in C
-sc_prohibit_semicolon_at_eol_in_python:
-	@prohibit='^[^#].*\;$$' \
-	in_vc_files='\.py$$' \
-	halt='python does not require to end lines with a semicolon' \
-	  $(_sc_search_regexp)
+sc_flake8:
+	@$(VC_LIST_EXCEPT) | $(GREP) '\.py$$' | xargs $(FLAKE8) --select E703 \
+		| $(GREP) . && { exit 1; } || :
 
 # mymain() in test files should use return, not exit, for nicer output
 sc_prohibit_exit_in_tests:
diff --git a/configure.ac b/configure.ac
index a8f8b051..93212ca7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -704,6 +704,10 @@ AC_PATH_PROGS([PYTHON], [python3 python2 python])
 if test -z "$PYTHON"; then
     AC_MSG_ERROR(['python3', 'python2' or 'python' binary is required to build libvirt])
 fi
+AC_PATH_PROG([FLAKE8], [flake8])
+if test -z "$FLAKE8"; then
+    AC_MSG_ERROR(['flake8' binary is required to check python code style])
+fi
 AC_PATH_PROG([PERL], [perl])
 if test -z "$PERL"; then
          AC_MSG_ERROR(['perl' binary is required to build libvirt])
-- 
2.17.1


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] maint: Use flake8 to check python code

Posted by Ján Tomko 4 years, 7 months ago

On Fri, Sep 13, 2019 at 01:37:20AM +0800, Shi Lei wrote:
>Replace 'sc_prohibit_semicolon_at_eol_in_python' with generic 'sc_flake8' rule
>to check python code style.
>
>Now 'sc_flake8' just check the error E703: 'statement ends with a semicolon'.
>In future, we could use '--select' to introduce more rules.
>

Nice, less language parsers using regexes in libvirt!

>Signed-off-by: Shi Lei <shi_lei@massclouds.com>
>---
> cfg.mk       | 8 +++-----
> configure.ac | 4 ++++
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
>diff --git a/cfg.mk b/cfg.mk
>index 42e1abf0..8acc45ac 100644
>--- a/cfg.mk
>+++ b/cfg.mk
>@@ -812,11 +812,9 @@ sc_require_enum_last_marker:
> 	    exit 1; } || :
>
> # In Python files we don't want to end lines with a semicolon like in C
>-sc_prohibit_semicolon_at_eol_in_python:
>-	@prohibit='^[^#].*\;$$' \
>-	in_vc_files='\.py$$' \
>-	halt='python does not require to end lines with a semicolon' \
>-	  $(_sc_search_regexp)
>+sc_flake8:
>+	@$(VC_LIST_EXCEPT) | $(GREP) '\.py$$' | xargs $(FLAKE8) --select E703 \

>+		| $(GREP) . && { exit 1; } || :

I don't think this line is necessary - we can just use the return value
from flake8

>
> # mymain() in test files should use return, not exit, for nicer output
> sc_prohibit_exit_in_tests:
>diff --git a/configure.ac b/configure.ac
>index a8f8b051..93212ca7 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -704,6 +704,10 @@ AC_PATH_PROGS([PYTHON], [python3 python2 python])
> if test -z "$PYTHON"; then
>     AC_MSG_ERROR(['python3', 'python2' or 'python' binary is required to build libvirt])
> fi
>+AC_PATH_PROG([FLAKE8], [flake8])
>+if test -z "$FLAKE8"; then
>+    AC_MSG_ERROR(['flake8' binary is required to check python code style])

While the error message is true, checking the code style is something
only developers should care about, so I'd make this a soft requirement
like we do with cppi.

Jano

>+fi
> AC_PATH_PROG([PERL], [perl])
> if test -z "$PERL"; then
>          AC_MSG_ERROR(['perl' binary is required to build libvirt])
>-- 
>2.17.1
>
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] maint: Use flake8 to check python code

Posted by Ján Tomko 4 years, 7 months ago

On Tue, Sep 17, 2019 at 05:37:24PM +0200, Ján Tomko wrote:
>On Fri, Sep 13, 2019 at 01:37:20AM +0800, Shi Lei wrote:
>>Replace 'sc_prohibit_semicolon_at_eol_in_python' with generic 'sc_flake8' rule
>>to check python code style.
>>
>>Now 'sc_flake8' just check the error E703: 'statement ends with a semicolon'.
>>In future, we could use '--select' to introduce more rules.
>>
>
>Nice, less language parsers using regexes in libvirt!
>
>>Signed-off-by: Shi Lei <shi_lei@massclouds.com>
>>---
>>cfg.mk       | 8 +++-----
>>configure.ac | 4 ++++
>>2 files changed, 7 insertions(+), 5 deletions(-)
>>
>>diff --git a/cfg.mk b/cfg.mk
>>index 42e1abf0..8acc45ac 100644
>>--- a/cfg.mk
>>+++ b/cfg.mk
>>@@ -812,11 +812,9 @@ sc_require_enum_last_marker:
>>	    exit 1; } || :
>>
>># In Python files we don't want to end lines with a semicolon like in C
>>-sc_prohibit_semicolon_at_eol_in_python:
>>-	@prohibit='^[^#].*\;$$' \
>>-	in_vc_files='\.py$$' \
>>-	halt='python does not require to end lines with a semicolon' \
>>-	  $(_sc_search_regexp)
>>+sc_flake8:
>>+	@$(VC_LIST_EXCEPT) | $(GREP) '\.py$$' | xargs $(FLAKE8) --select E703 \
>
>>+		| $(GREP) . && { exit 1; } || :
>
>I don't think this line is necessary - we can just use the return value
>from flake8
>

also, the libvirt-dbus invocation is using --show-source to be more
helpful

Jano

>>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list