Lior Vernia has posted comments on this change.

Change subject: webadmin: Allow editing labels on a host interface
......................................................................


Patch Set 7:

(3 comments)

http://gerrit.ovirt.org/#/c/22973/7/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostSetupNetworksModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostSetupNetworksModel.java:

Line 341:              * Interface Dialog
Line 342:              *******************/
Line 343:             final VdsNetworkInterface entity = 
((NetworkInterfaceModel) item).getEntity();
Line 344:             final HostNicModel interfacePopupModel = new 
HostNicModel(entity, getFreeLabels(), labelToIface);
Line 345:             editPopup = interfacePopupModel;
> isn't it sufficient to have a single variable?
Yes, if I keep casting it to HostNicModel. editPopup itself, which is used 
later with setConfirmWindow(), is just Model.
Line 346: 
Line 347:             // OK Target
Line 348:             okTarget = new BaseCommandTarget() {
Line 349:                 @Override


http://gerrit.ovirt.org/#/c/22973/7/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/NicLabelModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/NicLabelModel.java:

Line 15: 
Line 16:     private final Collection<VdsNetworkInterface> srcIfaces;
Line 17:     private final Collection<String> suggestedLabels;
Line 18:     private final Map<String, String> labelToIface;
Line 19:     private final Collection<String> originalLabels;
> I'd add some comments here for describing the purpose of each collection.
Done
Line 20:     private final Set<String> containedIfaces;
Line 21:     private final Set<String> flushedLabels;
Line 22: 
Line 23:     public Collection<String> getSuggestedLabels() {


Line 69:         }
Line 70:         return res;
Line 71:     }
Line 72: 
Line 73:     private void flush() {
> * IIUC, the purpose of the method is to flush the data from items to model?
Correct, and done.
Line 74:         flushedLabels.clear();
Line 75:         for (ListModel<String> labelModel : getItems()) {
Line 76:             flushedLabels.add(labelModel.getSelectedItem());
Line 77:         }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I030cbb7bcef1b05f329f67fc8a30ec5dee4b67f9
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Lior Vernia <[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