Ayal Baron has posted comments on this change.
Change subject: core: Remove disk from snapshots (#828192)
......................................................................
Patch Set 18: I would prefer that you didn't submit this
(5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java
Line 115: while (!imageTemplate.equals(currentGuid) &&
!currentGuid.equals(Guid.Empty)) {
you're inconsistent here.
I think it would be most readable if you keep currentGuid as first argument
i.e. !currentGuid.equals(imageTemplate) &&
Line 159: }
I think you're missing a space here (with vim this wouldn't have happened ;)
Line 161: private List<Snapshot>
prepareSnapshotConfigurationsWithoutImage(Guid imageGroupToRemove) {
s/Configuration/Config/? (shorter yet unambiguous and clear)
Line 163: List<DiskImage> snapshotDisks =
getDiskImageDao().getAllSnapshotsForImageGroup(imageGroupToRemove);
RemoveSnapshot above calls RemoveSnapshotFromDB
RemoveSnapshotFromDB calls getAllSnapshotsForImageGroup
so first, I don't think we should go the DB twice for the same info in same flow
second, it looks to me like this logic should be moved there
....................................................
Commit Message
Line 11: When a disk is removed all it's snapshots are removed.
s/it's/its/
--
To view, visit http://gerrit.ovirt.org/6025
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I97dedb9b8e8cd4dd0cae6c7b01d4b5678ba0a2d0
Gerrit-PatchSet: 18
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
