Gilad Chaplik has posted comments on this change. Change subject: webadmin: manage gluster swift services ......................................................................
Patch Set 16: Looks good to me, approved (3 inline comments) minor comments, you can address them in a future clean-up patch. .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterGeneralModel.java Line 247: query Line 248: ); Line 249: } Line 250: Line 251: private void manageGlusterSwiftServices() { when you have the time, I'd suggest to move the logic code to the popup-model (ManageGlusterSwiftModel). Line 252: if (getWindow() != null || getEntity() == null) { Line 253: return; Line 254: } Line 255: Line 395: } Line 396: } Line 397: } Line 398: Line 399: private GlusterSwiftAction getGlusterSwiftAction(GlusterServiceStatus currentStatus, currentStatus isn't used, is it needed? just in case you're missing a bug. Line 400: boolean isStart, Line 401: boolean isStop, Line 402: boolean isRestart) { Line 403: GlusterSwiftAction action = null; Line 803: Line 804: public static enum ClusterType { Line 805: GLUSTER, VIRT, BOTH Line 806: } Line 807: don't you prefer constants here? Line 808: public static enum GlusterSwiftAction { Line 809: START, Line 810: STOP, Line 811: RESTART -- To view, visit http://gerrit.ovirt.org/15263 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I43681486dd8f1e1d380a68c45f2e094a5c128ade Gerrit-PatchSet: 16 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches