Nir Soffer has posted comments on this change.

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


Patch Set 3:

(6 comments)

Nice, just need to cleanup up the tabs and fix the quoting.

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

Line 3: skipsdist = true
Line 4: [testenv:py27]
Line 5: deps =
Line 6:         pyflakes==0.9.2
Line 7:         pep8==1.5.6
We don't need tabs, this is not a makefile.
Line 8: commands=


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

Line 19: PEP8_BLACKLIST=(config.py \
Line 20:                constants.py \
Line 21:                crossImportsTests.py \
Line 22:                vdsm.py \
Line 23:               )
Tabs
Line 24: 
Line 25: if [ 'pyflakes' == $1 ]; then
Line 26:        (find . -path './.tox' -prune -type f -o \
Line 27:            -path './.git' -prune -type f -o \


Line 21:                crossImportsTests.py \
Line 22:                vdsm.py \
Line 23:               )
Line 24: 
Line 25: if [ 'pyflakes' == $1 ]; then
Should use single "=", see man test.

And $1 must be quoted.
Line 26:        (find . -path './.tox' -prune -type f -o \
Line 27:            -path './.git' -prune -type f -o \
Line 28:            -name '*.py' -o -name '*.py.in' && echo "${WHITELIST[@]}") 
| \
Line 29:            xargs pyflakes | grep -w -v "${SKIP_PYFLAKES_ERR}" | \


Line 26:        (find . -path './.tox' -prune -type f -o \
Line 27:            -path './.git' -prune -type f -o \
Line 28:            -name '*.py' -o -name '*.py.in' && echo "${WHITELIST[@]}") 
| \
Line 29:            xargs pyflakes | grep -w -v "${SKIP_PYFLAKES_ERR}" | \
Line 30:            while read LINE; do echo "$LINE"; false; done
Please covert the tabs to spaces, they were needed only in the makefile.
Line 31: fi
Line 32: 
Line 33: if [ 'pep8' == $1 ]; then
Line 34:        for x in ${PEP8_BLACKLIST[@]}; do \


Line 29:            xargs pyflakes | grep -w -v "${SKIP_PYFLAKES_ERR}" | \
Line 30:            while read LINE; do echo "$LINE"; false; done
Line 31: fi
Line 32: 
Line 33: if [ 'pep8' == $1 ]; then
Same
Line 34:        for x in ${PEP8_BLACKLIST[@]}; do \
Line 35:        exclude="$${exclude},$${x}" ; \
Line 36:                done ; \
Line 37:                pep8 --exclude="$${exclude}" --exclude='.tox/*' \


Line 35:        exclude="$${exclude},$${x}" ; \
Line 36:                done ; \
Line 37:                pep8 --exclude="$${exclude}" --exclude='.tox/*' \
Line 38:                --filename '*.py,*.py.in' . \
Line 39:                "${WHITELIST[@]}"
Tabs


-- 
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to