Alon Bar-Lev has posted comments on this change.

Change subject: wip: remove the use of @LockIdNameAttribute
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.ovirt.org/#/c/25944/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ActivateVdsCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ActivateVdsCommand.java:

Line 48:     }
Line 49: 
Line 50:     @Override
Line 51:     protected LockProperties getLockingPropertiesSettings() {
Line 52:         return new LockProperties();
if it is default, why override?
Line 53:     }
Line 54: 
Line 55:     @Override
Line 56:     protected void executeCommand() {


http://gerrit.ovirt.org/#/c/25944/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsSpmIdCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsSpmIdCommand.java:

Line 36:     }
Line 37: 
Line 38:     @Override
Line 39:     protected LockProperties getLockingPropertiesSettings() {
Line 40:         return new LockProperties().setWait(true);
this is ok
Line 41:     }
Line 42: 
Line 43:     @Override
Line 44:     protected boolean canDoAction() {


http://gerrit.ovirt.org/#/c/25944/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java:

Line 1751:      * to provide custom locking property settings
Line 1752:      */
Line 1753:     protected LockProperties getLockingPropertiesSettings() {
Line 1754:         return null;
Line 1755:     }
do not return null here to avoid extra logic.

always return object, and drop the null requirements all over.

there should be default lock properties object that state "do not lock".
Line 1756: 
Line 1757:     protected LockProperties getLockProperties() {
Line 1758:         LockProperties lockProperties = 
_parameters.getLockProperties();
Line 1759:         if (lockProperties == null) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie57e4f7c00ebcd6a4e9e0e61b7d26f50f2d00858
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Ravi Nori <[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