Lior Vernia 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 more important (and also would be consistent with your patch for the cluster/networks subtab). Also, same comment as in the previous patch, consider ternary operator. 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 accordance with my previous comment, should likely take precedence over display and migration. (by the way, it's weird that I implemented this here but not in the other subtab - but not related to your changes) 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 <yzasp...@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
