Einav Cohen has posted comments on this change.

Change subject: UI: change Make-Template to more readable text (#848104)
......................................................................


Patch Set 4: (2 inline comments)

....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java
Line 669:                 new AddVmTemplateParameters(newvm,
Line 670:                         (String) model.getName().getEntity(),
Line 671:                         (String) model.getDescription().getEntity());
Line 672: 
Line 673:         addVmTemplateParameters.setPublicUse(!(Boolean) 
model.getIsTemplatePublic().getEntity());
Assuming that the meaning of the "make public" check-box is the same as the 
"IsTemplatePublic" property of the model, then derez is correct - code should 
be:

parameters.isPublic = model.isPublic

and not:

parameters.isPublic = ! model.isPublic

Also, in one of the previous patch-sets I mentioned that "parameters.isPublic = 
!model.isPublic" is very confusing, so it shouldn't be that way in any case.

So it seems that the only thing that needs to be done here is to remove the 
negation, however need to verify that the behavior is correct after that change.
Line 674: 
Line 675:         if (model.getQuota().getSelectedItem() != null) {
Line 676:             newvm.setQuotaId(((Quota) 
model.getQuota().getSelectedItem()).getId());
Line 677:         }


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
Line 1645:         AddVmTemplateParameters addVmTemplateParameters =
Line 1646:                 new AddVmTemplateParameters(newvm,
Line 1647:                         (String) model.getName().getEntity(),
Line 1648:                         (String) model.getDescription().getEntity());
Line 1649:         addVmTemplateParameters.setPublicUse(!(Boolean) 
model.getIsTemplatePublic().getEntity());
same comments as in UserPortalListModel.
Line 1650: 
Line 1651:         addVmTemplateParameters.setDiskInfoDestinationMap(
Line 1652:                 model.getDisksAllocationModel()
Line 1653:                         .getImageToDestinationDomainMap((Boolean) 
model.getDisksAllocationModel()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9055763d66b09da175810ad3ba2ace0ff8316264
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shav...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to