Vojtech Szocs has posted comments on this change. Change subject: webadmin : Volume main and sub tabs' columns sortable ......................................................................
Patch Set 4: (7 comments) Patch looks good but please see my inline comments - I suggest to use LexoNumericComparator for comparing String values. http://gerrit.ovirt.org/#/c/28233/4/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/gluster/GlusterVolumeConditionFieldAutoCompleter.java File backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/gluster/GlusterVolumeConditionFieldAutoCompleter.java: Line 25: */ Line 26: public class GlusterVolumeConditionFieldAutoCompleter extends BaseConditionFieldAutoCompleter { Line 27: public final static GlusterVolumeConditionFieldAutoCompleter INSTANCE = new GlusterVolumeConditionFieldAutoCompleter(); Line 28: Line 29: public static enum FIELDS { Nested enum types are implicitly static. Despite that Java permits "static" modifier here, I'd just suggest to avoid it: public enum FIELDS { ... } Line 30: NAME, Line 31: TYPE, Line 32: TRANSPORT_TYPE, Line 33: REPLICA_COUNT, http://gerrit.ovirt.org/#/c/28233/4/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/gluster/SubTabVolumeBrickView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/gluster/SubTabVolumeBrickView.java: Line 59: }; Line 60: serverColumn.makeSortable(new Comparator<GlusterBrickEntity>() { Line 61: @Override Line 62: public int compare(GlusterBrickEntity brick0, GlusterBrickEntity brick1) { Line 63: return brick0.getServerName().compareTo(brick1.getServerName()); When comparing String values, it's advisable to use LexoNumericComparator, for example: serverColumn.makeSortable(new Comparator<GlusterBrickEntity>() { private final LexoNumericComparator lexoNumeric = new LexoNumericComparator(); @Override public int compare(GlusterBrickEntity brick0, GlusterBrickEntity brick1) { 62 return lexoNumeric.compare(brick0.getServerName(), brick1.getServerName()); } }); Line 64: } Line 65: }); Line 66: getTable().addColumn(serverColumn, constants.serverVolumeBrick(), "300px"); //$NON-NLS-1$ Line 67: Line 73: }; Line 74: directoryColumn.makeSortable(new Comparator<GlusterBrickEntity>() { Line 75: @Override Line 76: public int compare(GlusterBrickEntity o1, GlusterBrickEntity o2) { Line 77: return o1.getBrickDirectory().compareTo(o2.getBrickDirectory()); Same as above, please consider using LexoNumericComparator for comparing String values. Line 78: } Line 79: }); Line 80: Line 81: getTable().addColumn(directoryColumn, constants.brickDirectoryVolumeBrick(), "400px"); //$NON-NLS-1$ http://gerrit.ovirt.org/#/c/28233/4/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/gluster/SubTabVolumeParameterView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/gluster/SubTabVolumeParameterView.java: Line 43: }; Line 44: optionKeyColumn.makeSortable(new Comparator<GlusterVolumeOptionEntity>() { Line 45: @Override Line 46: public int compare(GlusterVolumeOptionEntity o1, GlusterVolumeOptionEntity o2) { Line 47: return o1.getKey().compareTo(o2.getKey()); Please consider using LexoNumericComparator for comparing String values. Line 48: } Line 49: }); Line 50: getTable().addColumn(optionKeyColumn, constants.optionKeyVolumeParameter(), "400px"); //$NON-NLS-1$ Line 51: TextColumnWithTooltip<GlusterVolumeOptionEntity> optionValueColumn = Line 57: }; Line 58: optionValueColumn.makeSortable(new Comparator<GlusterVolumeOptionEntity>() { Line 59: @Override Line 60: public int compare(GlusterVolumeOptionEntity o1, GlusterVolumeOptionEntity o2) { Line 61: return o1.getValue().compareTo(o2.getValue()); Please consider using LexoNumericComparator for comparing String values. Line 62: } Line 63: }); Line 64: getTable().addColumn(optionValueColumn, constants.optionValueVolumeParameter(), "400px"); //$NON-NLS-1$; Line 65: http://gerrit.ovirt.org/#/c/28233/4/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/host/SubTabHostBrickView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/host/SubTabHostBrickView.java: Line 38: }; Line 39: volNameColumn.makeSortable(new Comparator<GlusterBrickEntity>() { Line 40: @Override Line 41: public int compare(GlusterBrickEntity o1, GlusterBrickEntity o2) { Line 42: return o1.getVolumeName().compareTo(o2.getVolumeName()); Please consider using LexoNumericComparator for comparing String values. Line 43: } Line 44: }); Line 45: getTable().addColumn(volNameColumn, constants.volumeName()); //$NON-NLS-1$ Line 46: Line 52: }; Line 53: brickDirColumn.makeSortable(new Comparator<GlusterBrickEntity>() { Line 54: @Override Line 55: public int compare(GlusterBrickEntity o1, GlusterBrickEntity o2) { Line 56: return o1.getBrickDirectory().compareTo(o2.getBrickDirectory()); Please consider using LexoNumericComparator for comparing String values. Line 57: } Line 58: }); Line 59: getTable().addColumn(brickDirColumn, constants.brickDirectoryBricks(), "220px"); //$NON-NLS-1$ Line 60: -- To view, visit http://gerrit.ovirt.org/28233 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4795251fc2a73eb347f1e891d6c5f40ed8a67e47 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: anmolbabu <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Einav Cohen <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: anmolbabu <[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
