Alon Bar-Lev has posted comments on this change. Change subject: Introducing configurator package in vdsm-tool ......................................................................
Patch Set 7: (1 comment) .................................................... File lib/vdsm/tool/configurator.py Line 120: if diskimage_gid not in groups: Line 121: sys.stdout.write("sanlock service requires restart\n") Line 122: else: Line 123: sys.stdout.write("sanlock service is already configured\n") Line 124: retval = 0 please decide if you writing procedural or exceptional. if you are writing exceptional, you can always throw an exception if there is error and avoid nesting. retval above will always be not equal to zero, so condition is void Line 125: Line 126: return retval Line 127: Line 128: -- To view, visit http://gerrit.ovirt.org/20100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@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