Lior Vernia has posted comments on this change.

Change subject: webadmin: Labeled Networks with vlan shown outside the label 
group
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.ovirt.org/#/c/25477/1/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 733:             if (nic.getBondName() != null) {
Line 734:                 continue;
Line 735:             }
Line 736: 
Line 737:             List<NetworkLabelModel> nicLabels = 
nicToLabels.get(nicName);
Both this variable and nicToLabels aren't required if the loop is moved here; 
nicToLabels was used to "remember" the mapping created earlier.
Line 738: 
Line 739:             // does this nic have any labels?
Line 740:             Set<String> labels = nic.getLabels();
Line 741:             if (labels != null) {


Line 736: 
Line 737:             List<NetworkLabelModel> nicLabels = 
nicToLabels.get(nicName);
Line 738: 
Line 739:             // does this nic have any labels?
Line 740:             Set<String> labels = nic.getLabels();
This variable could be named nicLabels and used later instead of the mapped 
list.
Line 741:             if (labels != null) {
Line 742:                 for (String label : labels) {
Line 743:                     labelToIface.put(label, nicName);
Line 744:                     NetworkLabelModel labelModel = 
networkLabelMap.get(label);


Line 740:             Set<String> labels = nic.getLabels();
Line 741:             if (labels != null) {
Line 742:                 for (String label : labels) {
Line 743:                     labelToIface.put(label, nicName);
Line 744:                     NetworkLabelModel labelModel = 
networkLabelMap.get(label);
networkLabelMap was renamed from labelMap in an earlier patch, you said that 
the name wasn't good. What was wrong with it? It maps from a label's name to 
its corresponding model. This name isn't better, it's just longer.
Line 745:                     if (labelModel != null) {
Line 746:                         // attach label networks to nic
Line 747:                         for (Iterator<LogicalNetworkModel> iter = 
labelModel.getNetworks().iterator(); iter.hasNext();) {
Line 748:                             LogicalNetworkModel networkModel = 
iter.next();


Line 755:                                 errorLabelNetworks.add(networkModel);
Line 756:                             }
Line 757:                         }
Line 758: 
Line 759:                         // attach label itself to nic
I think this part isn't required anymore either.
Line 760:                         if (nicLabels == null) {
Line 761:                             nicLabels = new 
ArrayList<NetworkLabelModel>();
Line 762:                             nicToLabels.put(nicName, nicLabels);
Line 763:                         }


Line 765:                     }
Line 766:                 }
Line 767:             }
Line 768: 
Line 769:             Collection<LogicalNetworkModel> nicNetworks = 
nicToNetwork.get(nicName);
I would move this line before the loop and use nicNetworks inside it instead of 
nicToNetwork.get(nicName).
Line 770:             List<VdsNetworkInterface> bondedNics = 
bondToNic.get(nicName);
Line 771:             NetworkInterfaceModel nicModel;
Line 772: 
Line 773:             if (bondedNics != null) {


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

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