anmolbabu has posted comments on this change.

Change subject: webadmin : Volume and Brick Capacity UI
......................................................................


Patch Set 27:

(8 comments)

http://gerrit.ovirt.org/#/c/22537/27/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 12:     public static enum TimeUnit {
Line 13: 
Line 14:         /*
Line 15:          * The weights of the units here are their equivalent number 
of previous units
Line 16:          */
> If you could store <seconds> instead <no.of.prev-units>, it will be easy in
By no.of.prev-units I meant its value in terms of its predecessor unit.i.e,
1 min =60 sec. So, in Minutes(2,60),no.of.prev_unit is 60 and prev_unit is 
seconds.Having all units expressed in seconds will introduce more complexities 
when you try to convert from say mins to hours.It will increse the number of 
division by atleast 1.
Line 17: 
Line 18:         Seconds(1 , 1) , //Base unit assumed here for all practical 
purposes is seconds.And 1 sec = 1 sec so weight = 1
Line 19:         Minutes(2 , 60) , //1 minute = 60s
Line 20:         Hours(3 , 60) , //1 hour = 60 mins


Line 106:             result.setSecond(TimeUnit.timeUnits.get(i).getFirst());
Line 107:             if(product > inValue) {
Line 108:                 /*
Line 109:                  * If product exceeds the value, undo last 
transaction(multiplication and increment in i).
Line 110:                  */
> multiplication is not really a rollback, you may loose some due to the rema
ok so,I will have to maintain one temporary variable so that roll back is 
without loss of precision.
Line 111:                 product /= TimeUnit.timeUnits.get(i).getSecond();
Line 112:                 i--;
Line 113:                 
result.setSecond(TimeUnit.timeUnits.get(i).getFirst());
Line 114:                 break;


http://gerrit.ovirt.org/#/c/22537/27/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml
File 
frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml:

Line 196:         <include name="common/utils/SimpleDependecyInjector.java"/>
Line 197:         <include name="common/utils/VersionStorageFormatUtil.java"/>
Line 198:         <include name="common/utils/ExternalId.java"/>
Line 199:         <include name="common/utils/SizeConverter.java"/>
Line 200:         <include name="common/utils/TimeUnitConverter.java"/>
> These changes are not required as you have moved these classes to frontend
Done
Line 201: 
Line 202:         <!-- Required by frontend -->
Line 203:               <include name="common/interfaces/SearchType.java" />
Line 204:               <include 
name="common/businessentities/FenceStatusReturnValue.java" />


http://gerrit.ovirt.org/#/c/22537/27/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java:

Line 1512: 
Line 1513:     public static void refreshGlusterVolumeCapacityInfo(AsyncQuery 
aQuery, Guid volumeId) {
Line 1514:         aQuery.setHandleFailure(true);
Line 1515:         
Frontend.getInstance().runAction(VdcActionType.RefreshGlusterVolumeDetails, new 
GlusterVolumeParameters(volumeId));
Line 1516:     }
> why does this require a query?
Done
Line 1517: 
Line 1518:     public static void getGlusterRemoveBricksStatus(AsyncQuery 
aQuery,
Line 1519:             Guid clusterId,
Line 1520:             Guid volumeId,


http://gerrit.ovirt.org/#/c/22537/27/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/volumes/VolumeListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/volumes/VolumeListModel.java:

Line 874:         }, this);
Line 875:     }
Line 876: 
Line 877:     public void refreshVolumeCapacities() {
Line 878:         AsyncDataProvider.refreshGlusterVolumeCapacityInfo(new 
AsyncQuery(this, new INewAsyncCallback() {
> this is supposed to be an action
Done
Line 879:             @Override
Line 880:             public void onSuccess(Object model, Object returnValue) {
Line 881:                 if(((VdcQueryReturnValue) 
returnValue).getSucceeded()) {
Line 882:                     refresh();


http://gerrit.ovirt.org/#/c/22537/27/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIMessages.java
File 
frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIMessages.java:

Line 2: 
Line 3: import java.util.Date;
Line 4: import java.util.List;
Line 5: 
Line 6: import org.ovirt.engine.core.common.utils.SizeConverter.SizeUnit;
> not relevant
Done
Line 7: 
Line 8: import com.google.gwt.i18n.client.Messages.DefaultMessage;
Line 9: 
Line 10: public interface UIMessages extends 
com.google.gwt.i18n.client.Messages {


http://gerrit.ovirt.org/#/c/22537/27/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationMessages.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationMessages.java:

Line 101: 
Line 102:     @DefaultMessage("{0} GB")
Line 103:     String rebalanceFileSizeGb(String size);
Line 104: 
Line 105:     @DefaultMessage("Last updated {0} {1} ago \nClick the icon to 
refresh manually")
> "refresh now"
Done
Line 106:     String volumeCapacityRefreshClockToolTip(long time, String 
timeUnit);


http://gerrit.ovirt.org/#/c/22537/27/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/column/VolumeCapacityClockCell.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/column/VolumeCapacityClockCell.java:

Line 29:     private ApplicationMessages messages = 
ClientGinjectorProvider.getApplicationMessages();
Line 30: 
Line 31:     private VolumeListModel volumeListModel;
Line 32: 
Line 33:     public VolumeCapacityClockCell(VolumeListModel model) {
> VolumeListModel should not be used here as it could be invalidated later, w
Done
Line 34:         super("click");//$NON-NLS-1$
Line 35:         this.volumeListModel = model;
Line 36:     }
Line 37: 


-- 
To view, visit http://gerrit.ovirt.org/22537
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I04151d78838c1398cff42c84104e21d61d9c7747
Gerrit-PatchSet: 27
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

Reply via email to