Ramesh N has posted comments on this change. Change subject: webadmin : GeoRep Config popup view ......................................................................
Patch Set 12: (3 comments) https://gerrit.ovirt.org/#/c/34217/12/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeGeoReplicationSessionConfigModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeGeoReplicationSessionConfigModel.java: Line 101: configDiff configsChanged? Line 103: currentConfig.getEntity().getSecond() "currentConfig.getEntity().getSecond()" can be extracted to a variable to improve the readability. https://gerrit.ovirt.org/#/c/34217/12/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 310: setConfig(geoRepConfigModel); : resetConfig(geoRepConfigModel); Why do u need to handle set and reset seperatly. Just loop over all all the configs, if the reset flag is set, then add it to reset list and if no reset but there is a change then add it to changed list and perform reset and set . With this approach both of this methods can be merged and u don't need this lock flan on the GlusterVolumeGeoReplicationSessionConfigModel. -- To view, visit https://gerrit.ovirt.org/34217 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I150946fe016d85cb378ed3d548eab5581321fbfe Gerrit-PatchSet: 12 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: Shubhendu Tripathi <[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
