Arik Hadas has posted comments on this change. Change subject: core: managed removal of memory volumes in negative flows ......................................................................
Patch Set 6: I don't agree. I think that referring to the parent command is better in term of readability because it is more expressive, it is easier to understand on which flow you are. Your argument regarding compatibility with commands that inherit RemoveVm is valid, I didn't think of that case, but it is a result of misused of inheritence - if you want to have such command, you shouldn't inherit RemoveVm. there's a famous saying that composition is better for reuse than inheritence, if you would use RemoveVm instead of inherit it - you won't have this problem and that's the right thing to do in terms of OOP (see http://en.wikipedia.org/wiki/Composition_over_inheritance). BTW, in your example of the command with extended logging - for it I would have used flag in the parameters because it doesn't add different flow for the command, it makes minor chance in the currrent flow and thus the command remain simple. But as we're already using inheritance all over in sake of reuse (and probably will continue to do so) and Omer agreed with you, I changed it. -- To view, visit http://gerrit.ovirt.org/23222 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4b270ec0e1ab41cae34459dde9f9cf47b1b5bdf Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
