Yair Zaslavsky has posted comments on this change.

Change subject: [WIP] core: Adding ClearExternalJobCommand
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ClearExternalJobCommand.java
Line 17: 
Line 18:     @Override
Line 19:     protected boolean canDoAction() {
Line 20:         boolean retValue = true;
Line 21:         if (getParameters().getJobId() != null) {
Another comment here - since some of the code is repetetive with 
AddExternalJob, I would consider having a validator class (a Pattern introduced 
by kolesnik, see StoragePoolValidator).
We can have one for Steps and Jobs as they are tightly coupled.
You can also consult with Moti about refactoring Mike and he did in canDoAction 
code of network commands - looks really good.
Line 22:             job = DbFacade.getInstance().getJobDao().get((Guid) 
getParameters().getJobId());
Line 23:             if (job == null) {
Line 24:                 retValue = false;
Line 25:                 // TODO message


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id1b95a094dc586e6ebbdacd44e0a034e91605498
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to