Alona Kaplan 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 her nicLabels is needed. It is passed to the InterfaceModels constructors. Removed nicToLabels. 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 I don't understand this comment... 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 tha The new name adds information that it's network and not nic label. For me, it is more readable. 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. Done 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 instea Done 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 <alkap...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@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
