Liron Aravot has posted comments on this change. Change subject: core: CommandBase - obtain child commands on endAction() ......................................................................
Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/41690/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java: Line 579: parameters.add(command.getParameters()); Line 580: } Line 581: } Line 582: Line 583: commandBase.getParameters().setImagesParameters(parameters); > but setting the ImagesParameters is relevant only for a small part of the c I do see your point, but can't we say the same about locks, jobs, endAction() method as all (which is relevant for only some of the commands) and so on? I do agree that this entire class could be design a bit differently, but in the current situation this code should be here IMO. when and if we'll decide to split the logic here into different child classes we can refactor that out as well - currently the commandbase provides a general infra for all the commands which can be looked at as an advantage (when you add execution of child command you can just rely on the code here) or as a disadvantage. Line 584: } Line 585: } Line 586: Line 587: protected boolean isEndSuccessfully() { -- To view, visit https://gerrit.ovirt.org/41690 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia563dc1b97e0df852e242420eca4f987a39fc2b9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
