Tal Nisan has posted comments on this change.

Change subject: core: Added RegisterDiskCommand and GetUnregisteredDisksQuery
......................................................................


Patch Set 4: Looks good to me, but someone else must approve

(1 inline comment)

Very minor suggestion comment, aside for that look ok, great job
Waiting for Michael to ack the REST side and it's +2

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQuery.java
Line 47:                     unregQueryParams);
Line 48:             if (unregQueryReturn.getSucceeded()) {
Line 49:                 unregisteredDisks.add((Disk) 
unregQueryReturn.getReturnValue());
Line 50:             } else {
Line 51:                 log.error("Could not get populated disk, reason: " + 
unregQueryReturn.getExceptionString());
You might want to consider changing the log to something more clear, I'm not 
sure people will understand what populated stands for, maybe unregistered disk?
Just a suggestion though, not a must
Line 52:             }
Line 53:         }
Line 54:         getQueryReturnValue().setReturnValue(unregisteredDisks);
Line 55:     }


--
To view, visit http://gerrit.ovirt.org/11783
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I82de498fd9a8e25ed9e1dc5776f2fdf0c35b46da
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Chris Morrissey <cmorr...@netapp.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Chris Morrissey <cmorr...@netapp.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to