Allon Mureinik has posted comments on this change.
Change subject: core: GetAllVmSnapshotsFromConfigurationByVmIdQuery
......................................................................
Patch Set 4: Code-Review+1
(3 comments)
See nits inline, nothing major.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotVmConfigurationHelper.java
Line 21: import org.ovirt.engine.core.utils.log.LogFactory;
Line 22:
Line 23: public class SnapshotVmConfigurationHelper {
Line 24:
Line 25: protected final Log log = LogFactory.getLog(getClass());
should be private static final
Line 26:
Line 27: public VM getVmFromConfiguration(String configuration, Guid vmId,
Guid snapshotId) {
Line 28: VM vm = null;
Line 29: if (configuration != null) {
Line 112: }
Line 113:
Line 114: public DiskImageDAO getDiskImageDao() {
Line 115: return DbFacade.getInstance().getDiskImageDao();
Line 116: }
All these getXYZDao methods should be protected.
Line 117:
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetAllVmSnapshotsFromConfigurationByVmIdQueryTest.java
Line 87: assertTrue("snapshot should be in the return value",
snapshots.contains(snapshot));
Line 88: assertEquals("there should be exactly one snapshot returned",
1, snapshots.size());
Line 89:
Line 90: assertTrue("snapshot should contain a list of diskImages with
2 disks",
Line 91: !snapshots.isEmpty() &&
snapshots.get(0).getDiskImages().size() == 2);
I'd break this in to two separate asserts.
Line 92: }
--
To view, visit http://gerrit.ovirt.org/22774
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23aa4de4d233fade33d2a5ea174a9a1802b49370
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@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
