Eli Mesika has posted comments on this change.

Change subject: core: using JPA for engine backup awareness
......................................................................


Patch Set 4: -Verified

(5 comments)

https://gerrit.ovirt.org/#/c/40091/4/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EngineBackupLog.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EngineBackupLog.java:

Line 76: 
       :         result = prime * result + (passed ? 0 : 1);
       :         result = prime * result + ((outputMessage == null) ? 0 : 
outputMessage.hashCode());
> See my comment on equals method
As I understood from Moti's comment, I leave it as is


Line 96: && this.passed == other.passed
       :                 && Objects.equals(outputMessage, other.outputMessage)
> while this claim makes sense, the entire system is based on generated-by-id
see previous answer


Line 110: @Override
> Could please put @Override on a separate line?
Done


Line 117: @Override
> Could please put @Override on a separate line?
Done


https://gerrit.ovirt.org/#/c/40091/4/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/EngineBackupLogDaoDbFacadeImpl.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/EngineBackupLogDaoDbFacadeImpl.java:

Line 17:     }
Line 18: 
Line 19:     @Override
Line 20:     public EngineBackupLog getLastSuccessfulEngineBackup(String 
dbName) {
Line 21:         List<EngineBackupLog> logs = 
multipleResults(entityManager.createNamedQuery("EngineBackupLog.getLatest", 
EngineBackupLog.class).setParameter("dbName", dbName));
> 1. Do you not detach the returned result intentionally?
Done
Line 22:         if (logs.isEmpty()) {
Line 23:             return null;
Line 24:         } else {
Line 25:             return logs.get(0);


-- 
To view, visit https://gerrit.ovirt.org/40091
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia0aa9c4d0c8c0a5f7b40a022d7670f30c5fd5b77
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Liran Zelkha <liran.zel...@gmail.com>
Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
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

Reply via email to