Sergey Gotliv has posted comments on this change. Change subject: engine: cleanup in RemoveSnapshotSingleDiskCommand ......................................................................
Patch Set 1: (3 comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskCommand.java Line 14: import org.ovirt.engine.core.common.vdscommands.VDSCommandType; Line 15: import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; Line 16: import org.ovirt.engine.core.compat.Guid; Line 17: import org.ovirt.engine.core.dal.job.ExecutionMessageDirector; Line 18: import org.springframework.util.CollectionUtils; Your assumption is wrong, that's exactly what I meant. Let's start from the beginning. My motivation is replace existing: list != null && list.size() > 0 With more simple utility method: isEmpty() Unfortunately, apache introduced CollectionUtils.isEmpty() in 3.2 version while we are using the old 3.1. Of course I can move forward to 3.2 and I assume it should be safe based on my previous experience with upgrades of apache commons, but still I want do that more carefully and honestly isEmpty() is not worth to take chances. Spring CollectionUtils.isEmpty is already used in our code by CloudInitHandler. So why not here? Line 19: Line 20: @InternalCommandAttribute Line 21: public class RemoveSnapshotSingleDiskCommand<T extends ImagesContainterParametersBase> extends BaseImagesCommand<T> { Line 22: public RemoveSnapshotSingleDiskCommand(T parameters) { Line 47: Line 48: return runVdsCommand(VDSCommandType.MergeSnapshots, params); Line 49: } Line 50: Line 51: private Guid createTask(VDSReturnValue vdsReturnValue, Guid storageDomainId) { It was a matter of dependencies. Currently this command is not depends on AsyncTaskCreationInfo, one import less, so I wanted to keep it that way. Line 52: Guid taskId = persistAsyncTaskPlaceHolder(VdcActionType.RemoveSnapshot); Line 53: String message = ExecutionMessageDirector.resolveStepMessage(StepEnum.MERGE_SNAPSHOTS, Line 54: getJobMessageProperties()); Line 55: Line 48: return runVdsCommand(VDSCommandType.MergeSnapshots, params); Line 49: } Line 50: Line 51: private Guid createTask(VDSReturnValue vdsReturnValue, Guid storageDomainId) { Line 52: Guid taskId = persistAsyncTaskPlaceHolder(VdcActionType.RemoveSnapshot); Done Line 53: String message = ExecutionMessageDirector.resolveStepMessage(StepEnum.MERGE_SNAPSHOTS, Line 54: getJobMessageProperties()); Line 55: Line 56: return super.createTask(taskId, vdsReturnValue.getCreationInfo(), VdcActionType.RemoveSnapshot, -- To view, visit http://gerrit.ovirt.org/18688 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe60ca192123553a2c5a2bac2f34b3e4d9a64f3b Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com> 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