Tomas Jelinek has posted comments on this change.

Change subject: frontend: made the blank template editable
......................................................................


Patch Set 5:

(4 comments)

https://gerrit.ovirt.org/#/c/37905/5/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/builders/vm/CoreUnitToVmBaseBuilder.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/builders/vm/CoreUnitToVmBaseBuilder.java:

Line 29:         
vm.setVncKeyboardLayout(model.getVncKeyboardLayout().getSelectedItem());
Line 30:         
vm.setSerialNumberPolicy(model.getSerialNumberPolicy().getSelectedSerialNumberPolicy());
Line 31:         
vm.setCustomSerialNumber(model.getSerialNumberPolicy().getCustomSerialNumber().getEntity());
Line 32:         vm.setBootMenuEnabled(model.getBootMenuEnabled().getEntity());
Line 33:         
vm.setSpiceFileTransferEnabled(model.getSpiceFileTransferEnabled().getEntity() 
!= null ? model.getSpiceFileTransferEnabled().getEntity() : false);
> please consider using Boolean.TRUE.equals(model.getSpiceFileTransferEnabled
Done
Line 34:         
vm.setSpiceCopyPasteEnabled(model.getSpiceCopyPasteEnabled().getEntity() != 
null ? model.getSpiceCopyPasteEnabled().getEntity() : false);
Line 35:         vm.setAutoConverge(model.getAutoConverge().getSelectedItem());
Line 36:         
vm.setMigrateCompressed(model.getMigrateCompressed().getSelectedItem());
Line 37:         
vm.setCustomProperties(model.getCustomPropertySheet().serialize());


Line 30:         
vm.setSerialNumberPolicy(model.getSerialNumberPolicy().getSelectedSerialNumberPolicy());
Line 31:         
vm.setCustomSerialNumber(model.getSerialNumberPolicy().getCustomSerialNumber().getEntity());
Line 32:         vm.setBootMenuEnabled(model.getBootMenuEnabled().getEntity());
Line 33:         
vm.setSpiceFileTransferEnabled(model.getSpiceFileTransferEnabled().getEntity() 
!= null ? model.getSpiceFileTransferEnabled().getEntity() : false);
Line 34:         
vm.setSpiceCopyPasteEnabled(model.getSpiceCopyPasteEnabled().getEntity() != 
null ? model.getSpiceCopyPasteEnabled().getEntity() : false);
> same Boolean.TRUE.equals could be applied
Done
Line 35:         vm.setAutoConverge(model.getAutoConverge().getSelectedItem());
Line 36:         
vm.setMigrateCompressed(model.getMigrateCompressed().getSelectedItem());
Line 37:         
vm.setCustomProperties(model.getCustomPropertySheet().serialize());
Line 38:     }


https://gerrit.ovirt.org/#/c/37905/5/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/ExistingBlankTemplateModelBehavior.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/ExistingBlankTemplateModelBehavior.java:

Line 59:                                 new 
SerialNumberPolicyVmBaseToUnitBuilder() {
Line 60:                                     @Override
Line 61:                                     protected boolean 
supported(ConfigurationValues feature, UnitVmModel model) {
Line 62:                                         return true;
Line 63:                                     }
> Looking at this anonymous inheritors to override builder configuration I th
good point, but instead of factory methods which would become awkward when you 
need to give them other builders as inputs I have created methods 
withEveryFeatureSupported() using a fluent interface.
Line 64:                                 }) {
Line 65:                             @Override
Line 66:                             protected boolean 
supported(ConfigurationValues feature, UnitVmModel model) {
Line 67:                                 return true;


https://gerrit.ovirt.org/#/c/37905/5/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java:

Line 2779: 
Line 2780:         // Minimum 'Physical Memory Guaranteed' is 1MB
Line 2781:         validateMemorySize(getMemSize(), Integer.MAX_VALUE, 1);
Line 2782:         boolean isTemplateBehavior = (getBehavior() instanceof 
TemplateVmModelBehavior) ||
Line 2783:                 (getBehavior() instanceof 
ExistingBlankTemplateModelBehavior);
> This check seems to be useful enough to be extracted into small helper meth
Done
Line 2784:         if (!isTemplateBehavior && getMemSize().getIsValid()) {
Line 2785:             validateMemorySize(getMinAllocatedMemory(), 
getMemSize().getEntity(), 1);
Line 2786:         }
Line 2787:         setValidTab(TabName.RESOURCE_ALLOCATION_TAB, 
getMinAllocatedMemory().getIsValid());


-- 
To view, visit https://gerrit.ovirt.org/37905
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I400401b8e110b4cd10404f6df7c8f8ea4cd56093
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Martin Betak <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to