Tomas Jelinek has posted comments on this change. Change subject: userportal, webadmin: Base-template and template-version comboboxes merged ......................................................................
Patch Set 2: (7 comments) http://gerrit.ovirt.org/#/c/36278/2/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/AbstractVmPopupWidget.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/AbstractVmPopupWidget.java: Line 1379: return applicationTemplates.typeAheadEmptyContent().asString(); Line 1380: } Line 1381: } Line 1382: Line 1383: please remove this blank Line 1384: protected void localize(CommonApplicationConstants constants) { Line 1385: // Tabs Line 1386: highAvailabilityTab.setLabel(constants.highAvailVmPopup()); Line 1387: resourceAllocationTab.setLabel(constants.resourceAllocVmPopup()); http://gerrit.ovirt.org/#/c/36278/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/TemplateWithVersion.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/TemplateWithVersion.java: Line 39: */ Line 40: public TemplateWithVersion(VmTemplate baseTemplate, VmTemplate templateVersion) { Line 41: if (!(baseTemplate != null Line 42: && templateVersion != null Line 43: && baseTemplate.getId().equals(templateVersion.getBaseTemplateId()))) { This has a bit too much negations for me :) Please rewrite to: if (baseTemplate == null || templateVersion == null || !baseTemplate.getId().equals(templateVersion.getBaseTemplateId())) { ... } Line 44: throw new IllegalArgumentException(); Line 45: } Line 46: this.baseTemplate = baseTemplate; Line 47: this.templateVersion = templateVersion; Line 40: public TemplateWithVersion(VmTemplate baseTemplate, VmTemplate templateVersion) { Line 41: if (!(baseTemplate != null Line 42: && templateVersion != null Line 43: && baseTemplate.getId().equals(templateVersion.getBaseTemplateId()))) { Line 44: throw new IllegalArgumentException(); please add some explanation into the exception Line 45: } Line 46: this.baseTemplate = baseTemplate; Line 47: this.templateVersion = templateVersion; Line 48: } http://gerrit.ovirt.org/#/c/36278/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewTemplateVmModelBehavior.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewTemplateVmModelBehavior.java: Line 50: { Line 51: super.initialize(systemTreeSelectedItem); Line 52: getModel().getVmInitEnabled().setEntity(vm.getVmInit() != null); Line 53: getModel().getVmInitModel().init(vm.getStaticData()); Line 54: //getModel().getTemplate().setIsChangable(false); please remove Line 55: Line 56: getModel().getVmType().setIsChangable(true); Line 57: getModel().getCopyPermissions().setIsAvailable(true); Line 58: http://gerrit.ovirt.org/#/c/36278/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewVmModelBehavior.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewVmModelBehavior.java: Line 88: // VmTemplate template = getModel().getTemplate().getSelectedItem(); Line 89: // if (template != null) { Line 90: // selectedTemplateChanged(template); Line 91: // } Line 92: // } please remove dead code Line 93: Line 94: @Override Line 95: public void templateWithVersion_SelectedItemChanged() { Line 96: TemplateWithVersion selectedTemplateWithVersion = getModel().getTemplateWithVersion().getSelectedItem(); Line 243: getModel().getMinAllocatedMemory() Line 244: .setEntity((int) (getModel().getMemSize().getEntity() * overCommitFactor)); Line 245: } Line 246: Line 247: // TODO jakub this reference cleanup ? Line 248: private void updateTemplate() Line 249: { Line 250: final DataCenterWithCluster dataCenterWithCluster = Line 251: getModel().getDataCenterWithClustersList().getSelectedItem(); http://gerrit.ovirt.org/#/c/36278/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/PoolModelBehaviorBase.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/PoolModelBehaviorBase.java: Line 197: { Line 198: } Line 199: Line 200: @Override Line 201: public void oSType_SelectedItemChanged() { // grep for this bug grep for this bug? Line 202: VmTemplate template = getModel().getTemplateWithVersion().getSelectedItem() == null Line 203: ? null Line 204: : getModel().getTemplateWithVersion().getSelectedItem().getTemplateVersion(); Line 205: Integer osType = getModel().getOSType().getSelectedItem(); -- To view, visit http://gerrit.ovirt.org/36278 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iec2b522adc72ed0040e79059c1cd514ae3f5ad34 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Jakub Niedermertl <jnied...@redhat.com> Gerrit-Reviewer: Jakub Niedermertl <jnied...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
