Oved Ourfali has posted comments on this change.

Change subject: webadmin: Add option to skip fencing of host if SD is active
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/30195/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterModel.java:

Line 1482:         getSkipFencingIfSDActiveEnabled().setIsChangable(supported);
Line 1483:         if (supported) {
Line 1484:             
getSkipFencingIfSDActiveEnabled().setEntity(getEntity().isSkipFencingIfSDActive());
Line 1485:         } else {
Line 1486:             getSkipFencingIfSDActiveEnabled().setEntity(false);
> Shouldn't it be setEnabled(false) ? or something similar?
Looking at the code more, the logic is that you always show it, but set/unset 
it according to the config value, and allow to change that only if it is indeed 
supported?

I think we should hide the setting, and the side-tab entirely, in case there 
are no available settings (in cluster 3.4 we might not have anything, so no 
need to show it).

In addition, if supported, the default should be set to true.
Line 1487:         }
Line 1488:     }
Line 1489: 
Line 1490:     private void populateCPUList(ClusterModel clusterModel, 
List<ServerCpu> cpus, boolean canChangeArchitecture)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaecc1662324ce3d3072c9e850fcf8026ccfab0b2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Peřina <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Martin Peřina <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[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