Kanagaraj M has posted comments on this change. Change subject: webadmin : Volume Capacity UI - Part2 ......................................................................
Patch Set 13: (7 comments) http://gerrit.ovirt.org/#/c/23502/13/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/utils/TimeUnitConverter.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/utils/TimeUnitConverter.java: Line 6: import java.util.List; Line 7: Line 8: import org.ovirt.engine.core.common.utils.Pair; Line 9: Line 10: public class TimeUnitConverter { Please write a junit for this TimeUnitConverterTest Line 11: Line 12: public static enum TimeUnit { Line 13: Line 14: /* Line 80: /* Line 81: * Iterate over the list starting from the outUnit all the way upto inUnit and divide the inValue Line 82: * by the weights of the units iterated so far. Line 83: */ Line 84: inValue /= TimeUnit.timeUnits.get(i-1).getSecond(); expand this to inValue = inValue / TimeUnit.timeUnits.get(i-1).getSecond(); for better readability Line 85: } Line 86: } else { Line 87: //The conversion is from a bigger unit to a smaller unit. Line 88: for(int i = outUnit.getOrder(); i < inUnit.getOrder(); i++ ) { Line 89: /* Line 90: * Iterate over the list starting from the outUnit all the way upto inUnit and multiply the inValue Line 91: * by the weights of the units iterated so far. Line 92: */ Line 93: inValue *= TimeUnit.timeUnits.get(i).getSecond(); same here Line 94: } Line 95: } Line 96: Pair<Long , TimeUnitConverter.TimeUnit> timePair = new Pair<Long, TimeUnitConverter.TimeUnit>(inValue, outUnit); Line 97: //The result is a pair of converted value and its unit. http://gerrit.ovirt.org/#/c/23502/13/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeGeneralModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeGeneralModel.java: Line 117: setNumOfBricks(entity.getBricks() != null ? Integer.toString(entity.getBricks().size()) : null); Line 118: setTransportTypes(entity.getTransportTypes()); Line 119: setVolumeFreeCapacity((entity.getAdvancedDetails() != null && entity.getAdvancedDetails().getCapacityInfo() != null) ?entity.getAdvancedDetails().getCapacityInfo().getFreeSize(): null); Line 120: setVolumeTotalCapacity((entity.getAdvancedDetails() != null && entity.getAdvancedDetails().getCapacityInfo() != null) ? entity.getAdvancedDetails().getCapacityInfo().getTotalSize() : null); Line 121: setVolumeUsedCapacity((entity.getAdvancedDetails() != null && entity.getAdvancedDetails().getCapacityInfo() != null) ?entity.getAdvancedDetails().getCapacityInfo().getUsedSize(): null); Can the above statements wrapped inside an 'if' block to avoid repeating the null checks? Line 122: } Line 123: Line 124: public Set<TransportType> getTransportTypes() { Line 125: return transportTypes; http://gerrit.ovirt.org/#/c/23502/13/frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/ApplicationConstants.java File frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/ApplicationConstants.java: Line 348: @DefaultStringValue("Total usage") Line 349: String tooltipTotalUsageLabel(); Line 350: Line 351: @DefaultStringValue("Could not fetch data \nClick the icon to refresh now") Line 352: String volumeCapacityNoData(); is this being used? http://gerrit.ovirt.org/#/c/23502/13/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java: Line 3549: @DefaultStringValue("Remove") Line 3550: String removeIscsiBond(); Line 3551: Line 3552: @DefaultStringValue("Could not fetch data \nClick the icon to refresh now") Line 3553: String volumeCapacityNoData(); is this being used? Line 3554: Line 3555: @DefaultStringValue("iSCSI Multipathing") Line 3556: String dataCenterIscsiMultipathingSubTabLabel(); http://gerrit.ovirt.org/#/c/23502/13/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/gluster/SubTabVolumeGeneralView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/gluster/SubTabVolumeGeneralView.java: Line 105: } Line 106: }); Line 107: } Line 108: Line 109: private void initMemoryLabel() { initCapacityLabel? Line 110: this.volumeTotalCapacity = new VolumeCapacityLabel<Long>(constants); Line 111: this.volumeFreeCapacity = new VolumeCapacityLabel<Long>(constants); Line 112: this.volumeUsedCapacity = new VolumeCapacityLabel<Long>(constants); Line 113: } -- To view, visit http://gerrit.ovirt.org/23502 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I983daf32966527cdfa7773e07aa055a24519fbe2 Gerrit-PatchSet: 13 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: anmolbabu <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Ramesh N <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: Shubhendu Tripathi <[email protected]> Gerrit-Reviewer: anmolbabu <[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
