On Thu, Sep 12, 2019 at 12:56:04PM +0200, Andrea Bolognani wrote:
On Thu, 2019-09-12 at 11:13 +0100, Daniel P. Berrangé wrote:
> On Thu, Sep 12, 2019 at 05:55:38PM +0800, Shi Lei wrote:
> > +AC_PATH_PROG([PEP8], [pep8])
> > +if test -z "$PEP8"; then
> > + AC_MSG_ERROR(['pep8' binary is required to check python code
style])
> > +fi
>
> Using pep8 is an interesting idea. Especially with my series to
> standardize on using python for all build scripts, it will be
> valuable to have much more advanced python style checks.
>
> The only thing I wonder about is whether its reasonable to make
> it a mandatory requirement or not, since it is a separate package
> from python itself we can't assume it is present I think. It is
> on the various Linux we care about and FreeBSD too, but I'm not
> seeing it for macOS via homebrew.
>
> Also on my host 'pep8' is a python2 impl, you need 'pep8-3' for
> the python3 impl. Except that when I run it, it warns that
> it is renamed to pycodestyle upstream and 'pep8' will be dropped
> in future.
>
> IOW, I think we'll need to check for existence of the first available
> bniary from the list in this order:
>
> pycodestyle-3 pycodestyle pycodestyle-2 pep8-3 pep8 pep8-2
FWIW, libvirt-dbus is using flake8 to achieve what I believe is
basically the same result, whereas virt-manager I think uses pylint
and pycodestlye.
I am not familiar enough with the Python ecosystem to be able to
compare the various linters, but it would IMHO make sense to at
least try to standardize on one or more of them and use them across
libvirt-related projects.
pep8 validates code style against published PEP style guidelines.
pyflakes does static analysis to detect code errors
flake8 is a wrapper that runs pep8 and pyflakes and does some
other stuff.
For just doing this semicolon check then pep8 is sufficient,
but for a more general approach, then flake8 makes more
sense. In that case we'd delete sc_prohibit_semicolon_at_eol_in_python
entirely, and simply have a generic 'sc_flake8' check that runs a
configured list of checks against all py code.
Regards,
Daniel
--
|:
https://berrange.com -o-
https://www.flickr.com/photos/dberrange :|
|:
https://libvirt.org -o-
https://fstop138.berrange.com :|
|:
https://entangle-photo.org -o-
https://www.instagram.com/dberrange :|