Alissa Bonas has posted comments on this change.
Change subject: [wip] core: refactor MoveOrCopy/RemoveImage commands and
parameters
......................................................................
Patch Set 2: (4 inline comments)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java
Line 18: private boolean addImageDomainMapping;
Line 19: private boolean forceOverride;
Line 20: private NGuid sourceDomainId;
Line 21: private Guid destImageGroupId;
Line 22: private boolean shouldAttemptToRevert;
I don't understand why both the enum and this boolean are needed.
Isn't "never" in enum is actually shouldAttemptToRevert = false?
Line 23: private PerformOperationStage performRevertDbOperationsStage;
Line 24:
Line 25: public MoveOrCopyImageGroupParameters() {
Line 26: }
Line 19: private boolean forceOverride;
Line 20: private NGuid sourceDomainId;
Line 21: private Guid destImageGroupId;
Line 22: private boolean shouldAttemptToRevert;
Line 23: private PerformOperationStage performRevertDbOperationsStage;
why the member name is in plural ? actually its name is a bit more clear than
the enum name itself - please consider improving enum's name...
Line 24:
Line 25: public MoveOrCopyImageGroupParameters() {
Line 26: }
Line 27:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RemoveImageParameters.java
Line 8: private static final long serialVersionUID = -7905125604587768041L;
Line 9:
Line 10: private DiskImage diskImage;
Line 11: private boolean removeFromDB;
Line 12: private PerformOperationStage performDbOperationsStage =
PerformOperationStage.EXECUTION;
the name of the variable is not correct: it has operations in plural, and Db
should be DB.
Also, is the EXECUTION value just default ? is it OK that someone can override
it with the "setXXX" method?
Line 13:
Line 14: public RemoveImageParameters(Guid imageId) {
Line 15: super(imageId, null);
Line 16: setForceDelete(false);
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/PerformOperationStage.java
Line 1: package org.ovirt.engine.core.common.businessentities;
Line 2:
Line 3: public enum PerformOperationStage {
It's not very clear why this enum is needed - please elaborate (and its name
and names of the values are not very clear too)
Also, which code is checking the value of the enum and deciding based on it
when to run it? (maybe it's there but I just missed it)
Line 4: EXECUTION, TASKS_ENDED, NEVER;
--
To view, visit http://gerrit.ovirt.org/12689
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I92036258512d385b7296e1f75d9dcebfdd129d4a
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Anonymous Coward #1000370
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
