Dan Kenigsberg has posted comments on this change. Change subject: utils: Properly handle int argument in tobool(). ......................................................................
Patch Set 3: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/57511/3/lib/vdsm/utils.py File lib/vdsm/utils.py: Line 413: if s is None: Line 414: return False Line 415: if type(s) == bool: Line 416: return s Line 417: if isinstance(s, basestring) and s.lower() == 'true': wow, that's an old bug! Line 418: return True Line 419: return bool(int(s)) Line 420: except ValueError: Line 421: return False PS3, Line 420: except ValueError: > this alone could deserve its own change +1. The former one-liner solves the bug in the function. making the try-except smaller is a different (though just as noble) task. I believe that you can (and should) make the try-except block span only over the "return" line. -- To view, visit https://gerrit.ovirt.org/57511 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1740f82f5fa5eb50c319b46ce428bbb9f8c0c15e Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky <tgole...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Tomas Golembiovsky <tgole...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@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
