Kanagaraj M has posted comments on this change.

Change subject: webadmin : Create + Delete Geo-rep session
......................................................................


Patch Set 43:

(6 comments)

https://gerrit.ovirt.org/#/c/29691/43/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeGeoRepCreateModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeGeoRepCreateModel.java:

Line 64: overrideSuggestedConfig
no need for this local variable.

You could make this more readable.


if(getShowEligibleVolumes().getEntity()) {
    getEligibleVolumes();
}
else {
    getVolumesForForceSessionCreate();
}


Line 163: GlusterVolumeGeoRepCreateModel.this.stopProgress();
        : 
        :                 VdcQueryReturnValue vdcReturnValue = 
(VdcQueryReturnValue) returnValue;
        :                 if(!vdcReturnValue.getSucceeded()) {
        :                     
setQueryFailureMessage(vdcReturnValue.getExceptionString());
        :                 } else {
        :                     List<GlusterVolumeEntity> eligibleVolumesList = 
(List<GlusterVolumeEntity>) vdcReturnValue.getReturnValue();
        :                     setRecommendationViolations(null);
        :                     setVolumeList(eligibleVolumesList);
        :                     
getSlaveClusters().setItems(GlusterVolumeGeoRepCreateModel.this.getClusterForVolumes(eligibleVolumesList));
        :                 }
These above lines are similar to what is there in 
getVolumesForForceSessionCreate(), is it possible to bring them to a single 
place?


Line 219: getShowEligibleVolumes().setEntity(false);
Isn't the default is false?


https://gerrit.ovirt.org/#/c/29691/43/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeGeoRepListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeGeoRepListModel.java:

Line 228: createNewGeoRepSession
createGeoRepSession()


https://gerrit.ovirt.org/#/c/29691/43/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/GlusterVolumeGeoRepCreateSessionPopupView.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/GlusterVolumeGeoRepCreateSessionPopupView.java:

Line 173: 
Line 174:     @Override
Line 175:     public void edit(final GlusterVolumeGeoRepCreateModel object) {
Line 176:         driver.edit(object);
Line 177:         
setSuggestedConfigViolations(object.getRecommendationViolations());
This is not required
Line 178: 
Line 179:         setUserGroupVisibility(false);
Line 180:     }
Line 181: 


Line 179:  setUserGroupVisibility(false);
This is should go to setVisibilities() function


-- 
To view, visit https://gerrit.ovirt.org/29691
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied7317d456bf66db9a7800ba7106f2e8ec66429c
Gerrit-PatchSet: 43
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: 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

Reply via email to