Martin Betak has posted comments on this change.

Change subject: webadmin: Run Once & Sysprep: mask and verify user password
......................................................................


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/39418/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/RunOnceModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/RunOnceModel.java:

Line 215:     {
Line 216:         privateSysPrepPassword = value;
Line 217:     }
Line 218: 
Line 219:     private EntityModel<String> privateSysPrepPasswordVerification;
Please in new code try to follow java conventions even if you were the only one 
in the entire file :-)

More specifically:
- do not prefix field names with 'private'
- put opening '{' on the same line
- it is probably not best practice to put even short methods with their bodies 
on the same line, like in setSysPrepPasswordVerification
Line 220: 
Line 221:     public EntityModel<String> getSysPrepPasswordVerification()
Line 222:     {
Line 223:         return privateSysPrepPasswordVerification;


https://gerrit.ovirt.org/#/c/39418/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmInitModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmInitModel.java:

Line 879:             vmInit.setCustomScript(getSysprepScript().getEntity());
Line 880:             
vmInit.setActiveDirectoryOU(getActiveDirectoryOU().getEntity());
Line 881:             if (getSysprepPasswordEnabled()) {
Line 882:                 
vmInit.setRootPassword(getSysprepAdminPassword().getEntity());
Line 883:                 
vmInit.setRootPassword(getSysprepAdminPassword().getEntity());
Is this duplicate setting of root password intended?
Line 884:             }
Line 885:             
vmInit.setPasswordAlreadyStored(getSysprepPasswordSet().getEntity());
Line 886:             vmInit.setOrgName(getSysprepOrgName().getEntity());
Line 887:         } else {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I72d9d993abd52759e159e41577dc1030173e028e
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud <smela...@redhat.com>
Gerrit-Reviewer: Martin Betak <mbe...@redhat.com>
Gerrit-Reviewer: Shmuel Leib Melamud <smela...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to