Lior Vernia has posted comments on this change.

Change subject: webadmin: Add MAC pools in DC dialog
......................................................................


Patch Set 12:

(3 comments)

http://gerrit.ovirt.org/#/c/27792/12/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterListModel.java:

Line 296:         model.setTitle(constants.editDataCenterTitle());
Line 297:         model.setHelpTag(HelpTag.edit_data_center);
Line 298:         model.setHashName("edit_data_center"); //$NON-NLS-1$
Line 299:         model.getName().setEntity(dataCenter.getName());
Line 300:         model.initSelectedMacPool();
> Why do you need this code here? VdcQueryType.GetAllMacPools- onSuccess hasn
How can you be sure that an asynchronous call hasn't returned yet?
Line 301: 
Line 302:         if (getSystemTreeSelectedItem() != null
Line 303:                 && getSystemTreeSelectedItem().getType() == 
SystemTreeItemType.DataCenter)
Line 304:         {


http://gerrit.ovirt.org/#/c/27792/12/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterModel.java:

Line 324:             }
Line 325:         }
Line 326:     }
Line 327: 
Line 328:     public void initSelectedMacPool() {
> Maybe should be called also on entity changed event?
It's definitely not necessary. I also think it's bad practice to use 
setEntity() in parallel to setSelectedItem(), so I'd prefer not to "support" it.
Line 329:         Collection<MacPool> allMacPools = 
getMacPoolListModel().getItems();
Line 330:         StoragePool dc = getEntity();
Line 331:         if (allMacPools != null && dc != null) {
Line 332:             Guid macPoolId = dc.getMacPoolId();


http://gerrit.ovirt.org/#/c/27792/12/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/datacenter/DataCenterPopupView.ui.xml
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/datacenter/DataCenterPopupView.ui.xml:

Line 26:                                                                
<e:ListModelListBoxEditor ui:field="versionEditor" />
Line 27:                                                                
<e:ListModelListBoxEditor ui:field="quotaEnforceTypeEditor" />
Line 28:                                                                
<ge:StringEntityModelTextBoxEditor ui:field="commentEditor" />
Line 29:                                                        </g:FlowPanel>
Line 30:                                                </t:content>
> Please format
Done
Line 31:                                        </t:DialogTab>
Line 32:                                </t:tab>
Line 33:                                <t:tab>
Line 34:                                        <t:DialogTab 
label="{constants.dataCenterMacPoolTab}">


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I338a5647ded866c6b7b07542e9a95ea3dc3a2594
Gerrit-PatchSet: 12
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Martin Mucha <[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