Alissa Bonas has posted comments on this change.

Change subject: [wip] core: refactor MoveOrCopy/RemoveImage commands and 
parameters
......................................................................


Patch Set 2:

The enum just doesn't seem right here to me.
Who (which module/component) is deciding when and if the revert should be done 
- meaning which component decides on the stage/enum value? And the revert 
boolean - who is deciding on that?
I would recommend a completely different approach (which I explain below)
Create an interface that the relevant commands need to implement, the interface 
will contain 2 methods for each execution stage  - execution and tasks ended 
stage (the one for "never" is meaningless), and they can receive the "revert" 
boolean. The command is the one to know in which stage is the best to perform 
the operation, thus the relevant method will be implemented and the other one 
can have an empty implementation.
The methods will be always called by the command in the appropriate stage, so 
when the impl is empty it won't perform anything - which is the same as 
checking "if" on the enum value, while when the impl isn't empty it will 
perform whatever is needed based on revert boolean.

--
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

Reply via email to