Nir Soffer has posted comments on this change. Change subject: Makefile: use tox to run make pep8 and pyflakes ......................................................................
Patch Set 14: Code-Review-1 (8 comments) I don't see an issue with requiring tox, it just like any other requirement (autoconf). We should document the requirement in HACKING or some other file. https://gerrit.ovirt.org/#/c/49952/14//COMMIT_MSG Commit Message: Line 14: Signed-off-by: Yeela Kaplan <ykap...@redhat.com> Line 15: Signed-off-by: Yaniv Bronhaim <ybron...@redhat.com> Line 16: Signed-off-by: Irit Goihman <igoih...@redhat.com> Line 17: Signed-off-by: Yaniv Bronhaim <ybron...@redhat.com> Line 18: Signed-off-by: Irit Goihman <igoih...@redhat.com> > duplicated, but I don't think is a real issue It is, please remove the duplicates. https://gerrit.ovirt.org/#/c/49952/14/Makefile.am File Makefile.am: Line 96: done; Line 97: Line 98: .PHONY: pyflakes Line 99: pyflakes: Line 100: tox -- 'pyflakes' Quotes unneeded Line 101: Line 102: .PHONY: pep8 Line 103: pep8: Line 104: tox -- 'pep8' Line 100: tox -- 'pyflakes' Line 101: Line 102: .PHONY: pep8 Line 103: pep8: Line 104: tox -- 'pep8' Quotes unneeded Line 105: Line 106: .PHONY: python3 Line 107: python3: all Line 108: if [ "$(PYTHON3_SUPPORT)" == "1" ]; then \ Line 107: python3: all Line 108: if [ "$(PYTHON3_SUPPORT)" == "1" ]; then \ Line 109: PYTHONDONTWRITEBYTECODE=1 $(PYTHON3) -m compileall \ Line 110: $(WHITELIST) \ Line 111: `find -name '*.py' -not -path './.tox/*'` \ You remove the path to search for, must be the first argument to find. Line 112: else \ Line 113: echo "Warning: skipping python3 syntax check"; \ Line 114: fi Line 115: https://gerrit.ovirt.org/#/c/49952/14/automation/check-patch.sh File automation/check-patch.sh: Line 4 Line 5 Line 6 Line 7 Line 8 Please remove, we install these via tox. Line 5: set -xe Line 6: Line 7: easy_install pip Line 8: pip install -U pep8==1.5.7 pyflakes==1.1.0 Line 9: pip install -U tox Please specify the minimal version Line 10: Line 11: ./autogen.sh --system --enable-hooks Line 12: Line 13: make check NOSE_WITH_COVERAGE=1 NOSE_COVER_PACKAGE="$PWD/vdsm,$PWD/lib" https://gerrit.ovirt.org/#/c/49952/14/tox.ini File tox.ini: Line 1: [tox] Line 2: envlist = py27 Line 3: skipsdist = true Please separate sections with empty line: [tox] xxx = yyy [toxenv:py27] ... Line 4: [testenv:py27] Line 5: deps = Line 6: pyflakes==1.0.0 Line 7: pep8==1.5.7 https://gerrit.ovirt.org/#/c/49952/14/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: ) Leftover tab from the makefile, please use only spaces. (only makefile requires 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 \ -- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Irit Goihman <igoih...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
