Yair Zaslavsky has posted comments on this change.

Change subject: core: introduce context pattern
......................................................................


Patch Set 21:

(6 comments)

http://gerrit.ovirt.org/#/c/28829/21/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java:

Line 454:     public VdcReturnValueBase endAction(VdcActionType actionType,
Line 455:             VdcActionParametersBase parameters,
Line 456:             CommandContext context) {
Line 457:         CommandBase<?> command =
Line 458:                 context != null ? 
CommandsFactory.createCommand(actionType, parameters, context.duplicate()) :
> context must be non null
Done
Line 459:                         CommandsFactory.createCommand(actionType, 
parameters);
Line 460:         return command.endAction();
Line 461:     }
Line 462: 


http://gerrit.ovirt.org/#/c/28829/21/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 121:     private List<QuotaConsumptionParameter> consumptionParameters;
Line 122:     /** Indicates whether the acquired locks should be released after 
the execute method or not */
Line 123:     private boolean releaseLocksAtEndOfExecute = true;
Line 124:     /** Object which is representing a lock that some commands will 
acquire */
Line 125:     // private EngineLock commandLock;
> ?
Done
Line 126: 
Line 127:     protected Log log = LogFactory.getLog(getClass());
Line 128: 
Line 129:     /** The context defines how to monitor the command and handle its 
compensation */


Line 126: 
Line 127:     protected Log log = LogFactory.getLog(getClass());
Line 128: 
Line 129:     /** The context defines how to monitor the command and handle its 
compensation */
Line 130:     private CommandContext context = null;
> final please
Done
Line 131: 
Line 132:     /** A map contains the properties for describing the job */
Line 133:     protected Map<String, String> jobProperties;
Line 134: 


http://gerrit.ovirt.org/#/c/28829/21/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/CommandContext.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/CommandContext.java:

Line 78: 
Line 79:     @Override
Line 80:     public Object clone() throws CloneNotSupportedException {
Line 81:         super.clone();
Line 82:         return this.duplicate();
> I still do not understand why duplicate does not call clone... especially n
without super.clone - findbugs fails.
i have changed duplicate to call clone.
Line 83: 
Line 84:     }
Line 85: 
Line 86:     private void setFromContext(CommandContext context) {


http://gerrit.ovirt.org/#/c/28829/21/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/QueryContext.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/QueryContext.java:

Line 1: package org.ovirt.engine.core.bll.context;
Line 2: 
Line 3: public class QueryContext {
Line 4: 
Line 5:     private EngineContext engineContext;
> final please
Done
Line 6: 
Line 7:     public QueryContext(EngineContext engineContext) {
Line 8:         this.engineContext = engineContext;
Line 9:     }


http://gerrit.ovirt.org/#/c/28829/21/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java:

Line 42:     private List<StorageDomain> domains = new ArrayList<>();
Line 43:     private List<LUNs> luns = new ArrayList<>();
Line 44: 
Line 45:     public UpdateStorageServerConnectionCommand(T parameters) {
Line 46:         super(parameters, null);
> why not just add a super which does not accept context? this will avoid pat
done
Line 47:     }
Line 48: 
Line 49:     @Override
Line 50:     protected boolean canDoAction() {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I310f5f77fff78b3232ee77fe63791425fd521516
Gerrit-PatchSet: 21
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@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