Frank Kobzik has posted comments on this change.

Change subject: frontend: RNG device sources reporting
......................................................................


Patch Set 38:

(5 comments)

http://gerrit.ovirt.org/#/c/22259/38/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 1693:                 && getGlusterHostPassword().getIsValid()
Line 1694:                 && (getIsImportGlusterConfiguration().getEntity() ? 
(getGlusterHostAddress().getIsValid()
Line 1695:                 && getGlusterHostPassword().getIsValid()
Line 1696:                 && 
getSerialNumberPolicy().getCustomSerialNumber().getIsValid()
Line 1697:                 && isFingerprintVerified()) : true);
> you miss something like:
Done
Line 1698: 
Line 1699:         setIsGeneralTabValid(generalTabValid);
Line 1700: 
Line 1701:         return generalTabValid && 
getCustomPropertySheet().getIsValid();


Line 1701:         return generalTabValid && 
getCustomPropertySheet().getIsValid();
Line 1702:     }
Line 1703: 
Line 1704:     private void validateRngRequiredSource() {
Line 1705:         Version cluVersion = getVersion().getSelectedItem();
> Please call it "clusterVersion" instead of "cluVersion".
ok ;)
Line 1706:         boolean rngSupportedForCluster = 
isRngSupportedForClusterVersion(cluVersion);
Line 1707: 
Line 1708:         
getRngRandomSourceRequired().setIsValid(rngSupportedForCluster || 
!getRngRandomSourceRequired().getEntity());
Line 1709:         
getRngHwrngSourceRequired().setIsValid(rngSupportedForCluster || 
!getRngHwrngSourceRequired().getEntity());


Line 1708:         
getRngRandomSourceRequired().setIsValid(rngSupportedForCluster || 
!getRngRandomSourceRequired().getEntity());
Line 1709:         
getRngHwrngSourceRequired().setIsValid(rngSupportedForCluster || 
!getRngHwrngSourceRequired().getEntity());
Line 1710:     }
Line 1711: 
Line 1712:     private boolean isRngSupportedForClusterVersion(Version ver) {
> s/ver/version
Done
Line 1713:         if (ver == null) {
Line 1714:             return false;
Line 1715:         }
Line 1716: 


http://gerrit.ovirt.org/#/c/22259/38/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java:

Line 679:         
addVmTemplateParameters.setSoundDeviceEnabled(model.getIsSoundcardEnabled().getEntity());
Line 680:         
addVmTemplateParameters.setConsoleEnabled(model.getIsConsoleDeviceEnabled().getEntity());
Line 681:         
addVmTemplateParameters.setCopyVmPermissions(model.getCopyPermissions().getEntity());
Line 682:         addVmTemplateParameters.setUpdateRngDevice(true);
Line 683:         
addVmTemplateParameters.setRngDevice(model.getIsRngEnabled().getEntity() ? 
model.generateRngDevice() : null);
> please call setRngDeviceToParams(model, addVmTemplateParameters); also here
after discussion we decided not to do so. setRngDeviceToParams in this class is 
only for param classes that extend VmManagementParametersBase.
Line 684:         if (model.getIsSubTemplate().getEntity()) {
Line 685:             
addVmTemplateParameters.setBaseTemplateId(model.getBaseTemplate().getSelectedItem().getId());
Line 686:             
addVmTemplateParameters.setTemplateVersionName(model.getTemplateVersionName().getEntity());
Line 687:         }


http://gerrit.ovirt.org/#/c/22259/38/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmModelBehaviorBase.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmModelBehaviorBase.java:

Line 124:     private void setRngAvailability() {
Line 125:         TModel model = getModel();
Line 126:         Set<VmRngDevice.Source> requiredRngSources = 
model.getSelectedCluster().getRequiredRngSources();
Line 127:         
model.getIsRngEnabled().setIsChangable(!requiredRngSources.isEmpty());
Line 128:         
model.getIsRngEnabled().setMessage(constants.rngNotSupportedByCluster());
> please set the messages only if the editors are disabled.
Done
Line 129:         
model.getRngPeriod().setIsChangable(!requiredRngSources.isEmpty());
Line 130:         
model.getRngPeriod().setMessage(constants.rngNotSupportedByCluster());
Line 131:         
model.getRngBytes().setIsChangable(!requiredRngSources.isEmpty());
Line 132:         
model.getRngBytes().setMessage(constants.rngNotSupportedByCluster());


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd065835d18575b686ee805b032662205b31c966
Gerrit-PatchSet: 38
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[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