Dan Kenigsberg has uploaded a new change for review.
Change subject: make check: add a pylint check ......................................................................
make check: add a pylint check
pylint reports of plenty errors in our code, some of them are only style issues in our code or code that we use, but some are real.
This patch follows Lukas Bednar suggestion, and starts adding pylint check gradually. Please add more modules as they are cleaned up.
If pylint slows down you build too much, circumvent it with
make check-local PYLINT=true
Change-Id: Ie03052473caec201aa8ade7929ef1fce5fbc24a4 Signed-off-by: Dan Kenigsberg danken@redhat.com --- M Makefile.am M configure.ac M debian/control A pylint_blacklist M vdsm.spec.in 5 files changed, 118 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/82/24382/1
diff --git a/Makefile.am b/Makefile.am index 9b1a3c9..1539459 100644 --- a/Makefile.am +++ b/Makefile.am @@ -92,6 +92,13 @@ echo "Missing $${i%%.in} in .gitignore"; exit 1; fi; \ done; \ fi; + all=`mktemp`; \ + find * -path './.git' -prune -type f -o \ + -name '*.py' -o -name '*.py.in' | LC_ALL=C sort > $$all ; \ + LC_ALL=C comm -2 -3 $$all pylint_blacklist | \ + xargs $(PYLINT) -E --msg-template=\ + "{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}" ; \ + rm $$all
all-local: \ vdsm.spec diff --git a/configure.ac b/configure.ac index 55879e7..dacb2e4 100644 --- a/configure.ac +++ b/configure.ac @@ -164,6 +164,11 @@ AC_MSG_WARN([python-pep8 not found]) fi
+AC_PATH_PROG([PYLINT], [pylint]) +if test "x$PYLINT" = "x"; then + AC_MSG_WARN([pylint not found]) +fi + # Checking for python-devel AC_PATH_PROG([PYTHON_CONFIG], [python-config]) if test "x$PYTHON_CONFIG" = "x"; then diff --git a/debian/control b/debian/control index 45c2b1c..526a9a2 100644 --- a/debian/control +++ b/debian/control @@ -15,6 +15,7 @@ pep8, psmisc (>= 22.6), pyflakes, + pylint, python (>=2.7.3), python-dev, python-cpopen (>= 1.3), diff --git a/pylint_blacklist b/pylint_blacklist new file mode 100644 index 0000000..5254349 --- /dev/null +++ b/pylint_blacklist @@ -0,0 +1,104 @@ +client/vdsClientGluster.py +lib/vdsm/SecureXMLRPCServer.py +lib/vdsm/ipwrapper.py +lib/vdsm/netinfo.py +lib/vdsm/tool/configurator.py +lib/vdsm/tool/load_needed_modules.py.in +lib/vdsm/tool/service.py +lib/vdsm/tool/transient.py +lib/vdsm/tool/upgrade.py +lib/vdsm/tool/upgrade_300_networks.py +lib/vdsm/tool/validate_ovirt_certs.py.in +lib/vdsm/tool/vdsm-id.py +lib/vdsm/vdscli.py.in +lib/yajsonrpc/__init__.py +tests/apiTests.py +tests/cPopenTests.py +tests/capsTests.py +tests/configNetworkTests.py +tests/fileUtilTests.py +tests/fileVolumeTests.py +tests/functional/momTests.py +tests/functional/networkTests.py +tests/functional/sosPluginTests.py +tests/functional/storageTests.py +tests/functional/supervdsmFuncTests.py +tests/functional/utils.py +tests/functional/virtTests.py +tests/fuserTests.py +tests/getAllVolumesTests.py +tests/gluster_cli_tests.py +tests/guestIFTests.py +tests/hooksTests.py +tests/ipwrapperTests.py +tests/iscsiTests.py +tests/jsonRpcTests.py +tests/ksmTests.py +tests/libvirtconnectionTests.py +tests/lsblkTests.py +tests/lvmTests.py +tests/main.py +tests/md_utils_tests.py +tests/miscTests.py +tests/mkimageTests.py +tests/monkeypatchTests.py +tests/mountTests.py +tests/netconfTests.py +tests/netconfpersistenceTests.py +tests/netinfoTests.py +tests/netmodelsTests.py +tests/outOfProcessTests.py +tests/parted_utils_tests.py +tests/permutationTests.py +tests/persistentDictTests.py +tests/remoteFileHandlerTests.py +tests/resourceManagerTests.py +tests/schemaTests.py +tests/sslTests.py +tests/storageMailboxTests.py +tests/tcTests.py +tests/transportWrapperTests.py +tests/utilsTests.py +tests/vdsClientTests.py +tests/vmTests.py +tests/volumeTests.py +vdsm/BindingXMLRPC.py +vdsm/caps.py +vdsm/clientIF.py +vdsm/configNetwork.py +vdsm/debugPluginClient.py +vdsm/gluster/api.py +vdsm/gluster/cli.py +vdsm/gluster/hooks.py +vdsm/gluster/services.py +vdsm/gluster/tasks.py +vdsm/guestIF.py +vdsm/mkimage.py +vdsm/momIF.py +vdsm/netconf/ifcfg.py +vdsm/netconf/iproute2.py +vdsm/netmodels.py +vdsm/sos/vdsm.py.in +vdsm/sourceRoute.py +vdsm/sourceRouteThread.py +vdsm/storage/blockSD.py +vdsm/storage/blockVolume.py +vdsm/storage/fileSD.py +vdsm/storage/fileVolume.py +vdsm/storage/glusterSD.py +vdsm/storage/glusterVolume.py +vdsm/storage/hsm.py +vdsm/storage/iscsi.py +vdsm/storage/localFsSD.py +vdsm/storage/misc.py +vdsm/storage/nfsSD.py +vdsm/storage/resourceManager.py +vdsm/storage/sd.py +vdsm/storage/sp.py +vdsm/storage/storageServer.py +vdsm/storage/volume.py +vdsm/supervdsm.py +vdsm/vm.py +vdsm/vmChannels.py +vdsm_reg/engine.py.in +vdsm_reg/register-to-engine.py diff --git a/vdsm.spec.in b/vdsm.spec.in index 7eff7c1..268a98e 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -94,6 +94,7 @@
%if !0%{?rhel} BuildRequires: pyflakes +BuildRequires: pylint BuildRequires: python-pep8 %endif
oVirt Jenkins CI Server has posted comments on this change.
Change subject: make check: add a pylint check ......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6317/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7207/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7100/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/343/ : FAILURE
Yaniv Bronhaim has posted comments on this change.
Change subject: make check: add a pylint check ......................................................................
Patch Set 1:
(2 comments)
http://gerrit.ovirt.org/#/c/24382/1//COMMIT_MSG Commit Message:
Line 13: check gradually. Please add more modules as they are cleaned up. Line 14: Line 15: If pylint slows down you build too much, circumvent it with Line 16: Line 17: make check-local PYLINT=true add it to readme as well, and i guess it should be PYLINT=False to disable it Line 18: Line 19: Change-Id: Ie03052473caec201aa8ade7929ef1fce5fbc24a4
http://gerrit.ovirt.org/#/c/24382/1/vdsm.spec.in File vdsm.spec.in:
Line 93: %endif Line 94: Line 95: %if !0%{?rhel} Line 96: BuildRequires: pyflakes Line 97: BuildRequires: pylint why don't we need to require that in rhel? Line 98: BuildRequires: python-pep8 Line 99: %endif Line 100: Line 101: %if 0%{?with_systemd}
Lukas Bednar has posted comments on this change.
Change subject: make check: add a pylint check ......................................................................
Patch Set 1:
+1 I am surprised that it had quick response time. I wish you patience to get pylint_blacklist empty. thanks.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: make check: add a pylint check ......................................................................
Patch Set 2: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7219/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6329/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7113/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/344/ : FAILURE
oVirt Jenkins CI Server has posted comments on this change.
Change subject: make check: add a pylint check ......................................................................
Patch Set 3: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7220/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6330/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7114/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/345/ : FAILURE
Nir Soffer has posted comments on this change.
Change subject: make check: add a pylint check ......................................................................
Patch Set 3:
(1 comment)
I think we should add pylint, but it should be disabled by default.
We need an easy way to run pylint on all files:
make pylint
When we learn how to ignore pylint false errors, we can enable it in check-local.
http://gerrit.ovirt.org/#/c/24382/3//COMMIT_MSG Commit Message:
Line 13: check gradually. Please add more modules as they are cleaned up. Line 14: Line 15: If pylint slows down you build too much, circumvent it with Line 16: Line 17: make check-local PYLINT=true This is smart, but evil :-)
We can use PYLINT=true in the implementation of the Makefile, but in the interface it should be someting PYLINT=1 or PYLINT=y. Line 18: Line 19: where `true` is the ever-successful shell built-in. Line 20: Line 21: Change-Id: Ie03052473caec201aa8ade7929ef1fce5fbc24a4
Itamar Heim has posted comments on this change.
Change subject: make check: add a pylint check ......................................................................
Patch Set 3:
ping
Itamar Heim has abandoned this change.
Change subject: make check: add a pylint check ......................................................................
Abandoned
no activity. please restore if relevant.
vdsm-patches@lists.fedorahosted.org