Maor Lipchuk has posted comments on this change.

Change subject: engine: Only iSCSI storage connections are viable for iSCSI Bond
......................................................................


Patch Set 1:

(2 comments)

Generally it looks good to me.
I tend to agree with derez's comment regarding the helper class for storage type

http://gerrit.ovirt.org/#/c/23720/1//COMMIT_MSG
Commit Message:

Line 4: Commit:     Sergey Gotliv <[email protected]>
Line 5: CommitDate: 2014-01-26 16:18:53 +0200
Line 6: 
Line 7: engine: Only iSCSI storage connections are viable for iSCSI Bond
Line 8: 
Please add more detail
Line 9: Change-Id: Iae30db6aa0c5d2989e9e31566d6555d70259799e


http://gerrit.ovirt.org/#/c/23720/1/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetStorageConnectionsByDataCenterIdAndStorageTypeQueryParameters.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetStorageConnectionsByDataCenterIdAndStorageTypeQueryParameters.java:

Line 2: 
Line 3: import org.ovirt.engine.core.common.businessentities.StorageType;
Line 4: import org.ovirt.engine.core.compat.Guid;
Line 5: 
Line 6: public class 
GetStorageConnectionsByDataCenterIdAndStorageTypeQueryParameters extends 
IdQueryParameters {
suggestion: I must say, it looks like the longest class name I saw in oVirt, 
maybe it is worth to just rename the query to something more shorter... maybe 
something like ...GetConnectionsByDataCenterAndStorageType... (We can figure 
out by the API we need to deliever DataCenter id instead Data Center)

but this is a matter of taste, not that crucial
Line 7: 
Line 8:     private static final long serialVersionUID = 3630182261969029480L;
Line 9: 
Line 10:     private StorageType storageType;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae30db6aa0c5d2989e9e31566d6555d70259799e
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to