Change in vdsm[master]: Makefile: use tox to run make pep8 and pyflakes

ykaplan at redhat.com ykaplan at redhat.com
Wed Dec 9 16:22:56 UTC 2015


Yeela Kaplan has posted comments on this change.

Change subject: Makefile: use tox to run make pep8 and pyflakes
......................................................................


Patch Set 1:

(10 comments)

https://gerrit.ovirt.org/#/c/49952/1/Makefile.am
File Makefile.am:

Line 73:        vdsm/storage/curl-img-wrap \
Line 74:        vdsm/storage/fc-scan \
Line 75:        vdsm-tool/vdsm-tool \
Line 76:        $(NULL)
Line 77: 
> keep tabs. if you declare WHITELIST here as environment variable I'm not su
I need WHITELIST in both places.. 
The tox.sh will use a different process to run.
Line 78: 
Line 79: .PHONY: gitignore
Line 80: gitignore:
Line 81: 	@echo "Checking that .in files are ignored..."


Line 94: 	done;
Line 95: 
Line 96: .PHONY: pyflakes-pep8
Line 97: pyflakes-pep8:
Line 98: 	tox
> Yeela, would it be hard to keep the status quo, of having to different targ
Done
Line 99: 
Line 100: .PHONY: python3
Line 101: python3:
Line 102: 	if [ -x "$(PYTHON3)" ]; then \


Line 102: 	if [ -x "$(PYTHON3)" ]; then \
Line 103: 	    PYTHONDONTWRITEBYTECODE=1 $(PYTHON3) -m compileall \
Line 104: 		$(WHITELIST) \
Line 105: 		`find . -path './.tox -prune \
Line 106:                 -name '*.py' -o -name '*.py.in'`; \
> We should stop looking for files everywhere and filter out tox. Instead, we
What we are trying to achieve in the scope of this patch is running pep8 and pyflakes the same way as we do now but just use a specific version.

Any other changes are unrelated to this and should be addressed in a different, following patch.

One change at a time.
Line 107: 	else \
Line 108: 	    echo "Warning: skipping python3 syntax check"; \
Line 109: 	fi
Line 110: 


https://gerrit.ovirt.org/#/c/49952/1/tox.ini
File tox.ini:

Line 2: envlist = py27
Line 3: skipsdist = true
Line 4: [testenv:py27]
Line 5: commands=
Line 6:         pip install pyflakes==0.9.2
> Yes, I was too lazy to point to the docs.
Done
Line 7:         python -c 'import pyflakes; print("pyflakes-%s" % pyflakes.__version__)'
Line 8: 
Line 9:         pip install pep8==1.5.6
Line 10:         python -c 'import pep8; print("pep8-%s" % pep8.__version__)'


Line 3: skipsdist = true
Line 4: [testenv:py27]
Line 5: commands=
Line 6:         pip install pyflakes==0.9.2
Line 7:         python -c 'import pyflakes; print("pyflakes-%s" % pyflakes.__version__)'
> We don't need this now since we require specific version. This log was need
Done
Line 8: 
Line 9:         pip install pep8==1.5.6
Line 10:         python -c 'import pep8; print("pep8-%s" % pep8.__version__)'
Line 11: 


Line 6:         pip install pyflakes==0.9.2
Line 7:         python -c 'import pyflakes; print("pyflakes-%s" % pyflakes.__version__)'
Line 8: 
Line 9:         pip install pep8==1.5.6
Line 10:         python -c 'import pep8; print("pep8-%s" % pep8.__version__)'
> Same
Done
Line 11: 


https://gerrit.ovirt.org/#/c/49952/1/tox.sh
File tox.sh:

Line 1: #!/bin/sh
> Lets create separate script for pep8 and pyflakes. We can run both of them 
Done
Line 2: 
Line 3: WHITELIST=(contrib/logdb \
Line 4:            contrib/profile-stats \
Line 5:            init/daemonAdapter \


Line 10:            vdsm/vdsm-restore-net-config \
Line 11:            vdsm/storage/curl-img-wrap \
Line 12:            vdsm/storage/fc-scan \
Line 13:            vdsm-tool/vdsm-tool
Line 14:           )
> try to avoid this duplication. maybe declare it in external location and lo
source won't work here since the makefile and shell script has different formats for lists... do you have another idea?
Line 15: 
Line 16: SKIP_PYFLAKES_ERR="\./vdsm/storage/lvm\.py.*: list comprehension redefines \
Line 17:        'lv' from line .*"
Line 18: 


Line 33: 	done ; \
Line 34: 	pep8 --exclude="$${exclude}" --exclude='.tox/*' \
Line 35:         --filename '*.py,*.py.in' . \
Line 36: 	"${WHITELIST[@]}"
Line 37
> tabs or not tabs  .. choose
Done


https://gerrit.ovirt.org/#/c/49952/1/vdsm.spec.in
File vdsm.spec.in:

Line 98: 
Line 99: %if 0%{?with_check}
Line 100: BuildRequires: pyflakes
Line 101: BuildRequires: python-pep8
Line 102: BuildRequires: python-tox
> isn't the *second* one enough?
Done
Line 103: %endif
Line 104: 
Line 105: BuildRequires: systemd-units
Line 106: 


-- 
To view, visit https://gerrit.ovirt.org/49952
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id583de2d411bb5bcd0f717f569d2961b555334c9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list