Yevgeny Zaspitsky has posted comments on this change. Change subject: webadmin: add management role indication to the network-cluster sub-tab ......................................................................
Patch Set 5: (2 comments) http://gerrit.ovirt.org/#/c/36911/5/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/network/SubTabNetworkClusterView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/network/SubTabNetworkClusterView.java: Line 147: } else { Line 148: images.add(emptyImage); Line 149: Line 150: } Line 151: if (object.getSecond().isManagement()) { > It would make more sense to have this role as the first one - it's much mor Done. Regarding ternary operators: 1. I'm not sure I'm in favor. (a style issue) 2. That isn't mine invention. 3. I guess it might/should be a separate patch. Line 152: images.add(managementImage); Line 153: } else { Line 154: images.add(emptyImage); Line 155: Line 177: Line 178: return NetworkRoleColumnHelper.getTooltip(imagesToText); Line 179: } Line 180: }; Line 181: netRoleColumn.makeSortable(new Comparator<PairQueryable<VDSGroup, NetworkCluster>>() { > How about updating the comparator to take management role into account? In Done Line 182: Line 183: private int calculateValue(NetworkCluster networkCluster) { Line 184: int res = 0; Line 185: if (networkCluster != null) { -- To view, visit http://gerrit.ovirt.org/36911 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e99610ca2e83e75e4d078e5dd785fd7d6104cec Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Yevgeny Zaspitsky <[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
