Change in vdsm[master]: Do not delete the template when zeroing a dependant image.

smizrahi at redhat.com smizrahi at redhat.com
Mon Feb 18 20:57:54 UTC 2013


Saggi Mizrahi has posted comments on this change.

Change subject: Do not delete the template when zeroing a dependant image.
......................................................................


Patch Set 1: I would prefer that you didn't submit this

Patch Set 2: I would prefer that you didn't submit this
I tend to agree with Federico's view that leaving the filtering to the function instead of to the caller ( http://gerrit.ovirt.org/#/c/12105/2 ) is the correct things to do.
A call to this method with incorrect filtering will leave the domain broken thus logic dictates that you need to validate the list internally anyway to ensure sanity (which we don't and as Federico noted is an invitation for bugs).
Federico's version show that there are really only two cases in which you should run the method whereas Eduardo's version make it hidden in comments and conventions that no one reads anyway.
IMO A comment should only exist when self explanatory code is impossible.

--
To view, visit http://gerrit.ovirt.org/12178
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9e472334efa9dadb5389db70b0953f88b9be858a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov at gmail.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list