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

Reply via email to