Maor Lipchuk has posted comments on this change.

Change subject: core: ImportVm cleanup: action and type params
......................................................................


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

(1 inline comment)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
Line 140: 
Line 141:         if (!retVal) {
Line 142:             addCanDoActionMessage(VdcBllMessages.VAR__ACTION__IMPORT);
Line 143:             addCanDoActionMessage(VdcBllMessages.VAR__TYPE__VM);
Line 144:         }
I think you better use setActionMessageParameters to add those parameter 
messages, instead add them it in the CanDoAction.
 The reason for that is that setActionMessageParameters is already called as 
part of the flow in MoveOrCopyTemplateCommand, and it sets the CDA list of 
parameter messages.
 So what will happened now is that, in this list, there will be two types of 
type, VM and Template. that could cause bug in the message by performing 
template instead of VM as the entity type (Which probably already happens now)
Line 145: 
Line 146:         return retVal;
Line 147:     }
Line 148: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b3148cd54146ac0b90924464cd27fb54160889b
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to