Irit Goihman has posted comments on this change. Change subject: tox: fail make process if required tox version isn't installed. ......................................................................
Patch Set 21: Verified+1 (7 comments) https://gerrit.ovirt.org/#/c/59306/20/Makefile.am File Makefile.am: Line 93 Line 94 Line 95 Line 96 Line 97 > I think we can simplify and make it more clear like this: Done https://gerrit.ovirt.org/#/c/59306/17/build-aux/vercmp File build-aux/vercmp: Line 26: Line 27: Line 28: def main(args): Line 29: args = parse_args() Line 30: if args.verbose: > You want to setup logging always. --verbose should effect only the log leve Done Line 31: level = logging.DEBUG Line 32: else: Line 33: level = logging.INFO Line 34: Line 28: def main(args): Line 29: args = parse_args() Line 30: if args.verbose: Line 31: level = logging.DEBUG Line 32: else: > Better exit in __main__, and return a value here, so we can test this funct Done Line 33: level = logging.INFO Line 34: Line 35: logging.basicConfig(level=level, format="%(name)s: %(message)s") Line 36: return compare_versions(args.actual_version, args.required_version) Line 40: actual_version = [int(n) for n in actual_version.split('.')] Line 41: required_version = [int(n) for n in required_version.split('.')] Line 42: Line 43: padding = len(actual_version) - len(required_version) Line 44: if padding > 0: > This will use the root logger, so we will see this output: Done Line 45: required_version += [0] * padding Line 46: Line 47: if actual_version < required_version: Line 48: log.debug("%s < %s", actual_version, required_version) https://gerrit.ovirt.org/#/c/59306/19/build-aux/vercmp File build-aux/vercmp: Line 44: if padding > 0: Line 45: required_version += [0] * padding Line 46: Line 47: if actual_version < required_version: Line 48: log.debug("%s < %s", actual_version, required_version) > You are not using the logger, so the output is: Done Line 49: return 0 Line 50: elif actual_version == required_version: Line 51: log.debug("%s == %s", actual_version, required_version) Line 52: return 1 https://gerrit.ovirt.org/#/c/59306/20/build-aux/vercmp File build-aux/vercmp: Line 39: def compare_versions(actual_version, required_version): Line 40: actual_version = [int(n) for n in actual_version.split('.')] Line 41: required_version = [int(n) for n in required_version.split('.')] Line 42: Line 43: padding = len(actual_version) - len(required_version) > This should be: Done Line 44: if padding > 0: Line 45: required_version += [0] * padding Line 46: Line 47: if actual_version < required_version: https://gerrit.ovirt.org/#/c/59306/17/tox.sh File tox.sh: Line 1: #!/bin/sh -e Line 2: Line 3: WHITELIST=(build-aux/vercmp \ Line 4: contrib/logdb \ > We use only spaces in scripts, this is not a makefile. Done Line 5: contrib/logstat \ Line 6: contrib/profile-stats \ Line 7: init/daemonAdapter \ Line 8: vdsm/get-conf-item \ -- To view, visit https://gerrit.ovirt.org/59306 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I665025dacdd5346a5e021ac98e864f7b6461917c Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Irit Goihman <igoih...@redhat.com> Gerrit-Reviewer: Edward Haas <edwa...@redhat.com> Gerrit-Reviewer: Irit Goihman <igoih...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yaniv Bronhaim <ybron...@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/admin/lists/vdsm-patches@lists.fedorahosted.org
