Eldan Shachar has posted comments on this change.

Change subject: userportal/webadmin: cluster parameters override - UI 
enhancements
......................................................................


Patch Set 3:

(7 comments)

https://gerrit.ovirt.org/#/c/38407/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java:

Line 4163:                 new VdsIdParametersBase(hostId),
Line 4164:                 aQuery);
Line 4165:     }
Line 4166: 
Line 4167:     public void verifyClusterSupportsVmCpu(AsyncQuery aQuery, Guid 
vdsGroupId, String newCpuName) {
> this is strangely named - from the name I would expect it to return a Boole
Done
Line 4168:         aQuery.converterCallback = new IAsyncConverter() {
Line 4169:             @Override
Line 4170:             public Object Convert(Object source, AsyncQuery 
_asyncQuery)
Line 4171:             {


Line 4167:     public void verifyClusterSupportsVmCpu(AsyncQuery aQuery, Guid 
vdsGroupId, String newCpuName) {
Line 4168:         aQuery.converterCallback = new IAsyncConverter() {
Line 4169:             @Override
Line 4170:             public Object Convert(Object source, AsyncQuery 
_asyncQuery)
Line 4171:             {
> in new code please use java formatting
Done
Line 4172:                 if (source != null)
Line 4173:                 {
Line 4174:                     return source;
Line 4175:                 }


https://gerrit.ovirt.org/#/c/38407/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/help/HelpTag.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/help/HelpTag.java:

Line 261:     migrate_virtual_machine("migrate_virtual_machine", 
HelpTagType.WEBADMIN, "VMs Tab > Migrate Virtual Machine(s)"), //$NON-NLS-1$ 
//$NON-NLS-2$
Line 262: 
Line 263:     edit_next_run_configuration("edit_next_run_configuration", 
HelpTagType.COMMON, "'VMs' main tab -> 'Edit VM' dialog [replaces the old 'Edit 
Desktop' (edit_desktop) and 'Edit Server' (edit_server)]"), //$NON-NLS-1$ 
//$NON-NLS-2$
Line 264: 
Line 265:     edit_unsupported_cpu("edit_unsupported_cpu", HelpTagType.COMMON, 
"'VMs' main tab -> 'Edit VM' dialog [replaces the old 'Edit Desktop' 
(edit_desktop) and 'Edit Server' (edit_server)]"), //$NON-NLS-1$ //$NON-NLS-2$
> the comment seems to be quite incorrect :)
Done
Line 266: 
Line 267:     monitor("monitor", HelpTagType.UNKNOWN), //$NON-NLS-1$
Line 268: 
Line 269:     move_disk("move_disk", HelpTagType.WEBADMIN, "VMs Tab > Virtual 
Disks Sub-Tab > Move Virtual Disk"), //$NON-NLS-1$ //$NON-NLS-2$


https://gerrit.ovirt.org/#/c/38407/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterListModel.java:

Line 707:                             } else {
Line 708:                                 
message.append(ConstantsManager.getInstance()
Line 709:                                         .getConstants()
Line 710:                                         
.changeCpuLevelWarningMessage());
Line 711:                                 
cpuLevelConfirmationWindow(message.toString());
> it takes two server calls until you get here which may take long on slow ne
The original window is still visible at this point, I wrapped the async task 
with start\stopProgress.
Line 712:                             }
Line 713:                         }
Line 714:                     };
Line 715:                     AsyncDataProvider.getInstance()


https://gerrit.ovirt.org/#/c/38407/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/pools/PoolListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/pools/PoolListModel.java:

Line 458: onSavePhase2
> this does not signify what does this do...
Done


https://gerrit.ovirt.org/#/c/38407/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/TemplateListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/TemplateListModel.java:

Line 629:                                                 .getConstants()
Line 630:                                                 
.nameMustBeUniqueInvalidReason());
Line 631:                                 model.getName().setIsValid(false);
Line 632:                                 
model.setValidTab(TabName.GENERAL_TAB, false);
Line 633:                                 return;
> this return is not needed here since it is the last line of the method
see comment below
Line 634:                             }
Line 635: 
Line 636:                         }
Line 637:                     }), name);


Line 635: 
Line 636:                         }
Line 637:                     }), name);
Line 638:         }
Line 639:         String selectedCpu = model.getCustomCpu().getSelectedItem();
> ...and this part is going to be executed in parallel to the name uniqueness
right, my bad, it was meant to be inside the async call (same as in 
PoolListModel) and not part of this flow of code.
Line 640:         if (selectedCpu != null && !selectedCpu.isEmpty()  && 
!model.getCustomCpu().getItems().contains(selectedCpu)) {
Line 641:             ConfirmationModel confirmModel = new ConfirmationModel();
Line 642:             
confirmModel.setTitle(ConstantsManager.getInstance().getConstants().vmUnsupportedCpuTitle());
Line 643:             
confirmModel.setMessage(ConstantsManager.getInstance().getConstants().vmUnsupportedCpuMessage());


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2e1c458dc4e4b6e61d2a1b92327287166a12afa2
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eldan Shachar <[email protected]>
Gerrit-Reviewer: Eldan Shachar <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to