Daniel Erez has posted comments on this change. Change subject: webadmin: Allow editing labels on a host interface ......................................................................
Patch Set 7: Code-Review+2 (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? 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. 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? * I'd add some comments describing the purpose of flush method, flushedLables, etc... 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 <lver...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> 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
