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