Nir Soffer has posted comments on this change. Change subject: build: Forbid bare except: ......................................................................
Patch Set 3: (2 comments) .................................................... File Makefile.am Line 78: SKIP_PYFLAKES_ERR = "\./vdsm/storage/lvm\.py.*: list comprehension redefines \ Line 79: 'lv' from line .*" Line 80: Line 81: check-local: Line 82: if git show | egrep '^\+\s+except:\s*(#.+)?$$'; then \ I don't have problem with "except Exception:" when you want to handle all exceptions. This is typical for any thread main loop. Line 83: echo "error: empty "except:" is forbidden!"; \ Line 84: exit 1; \ Line 85: fi Line 86: find . -path './.git' -prune -type f -o \ Line 78: SKIP_PYFLAKES_ERR = "\./vdsm/storage/lvm\.py.*: list comprehension redefines \ Line 79: 'lv' from line .*" Line 80: Line 81: check-local: Line 82: if git show | egrep '^\+\s+except:\s*(#.+)?$$'; then \ pyflakes may have this feature soon: https://github.com/pyflakes/pyflakes/pull/22, eliminating the need for this patch. I'm afraid that we cannot fix the issue of checking other people code. Anything we do (git diff, git diff --cached, git show...) will fail in some way or another. For example, if you committed, git show is correct, if you did not commit, git diff is correct. If you added files to index, git diff is not correct. This is mess. There is simple solution: - Fix all 173 bare excepts (use except BaseException: as start, making this safe) - Check all code like we do in current pyflakes/pep8 Later can can convert BaseException to something more appropriate. Line 83: echo "error: empty "except:" is forbidden!"; \ Line 84: exit 1; \ Line 85: fi Line 86: find . -path './.git' -prune -type f -o \ -- To view, visit http://gerrit.ovirt.org/22456 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5b7f33b6eb1c90610b3a5d053ba65ce3fc6b74fa Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
