Alissa Bonas has posted comments on this change.

Change subject: core,restapi: Alias for registered disk
......................................................................


Patch Set 1: (1 inline comment)

A general concern about the patch - having more than one disk with empty alias, 
will lead to generate them the same alias, and then in UI it will be impossible 
to distinguish them. Perhaps I missed it , but is it treated here?
If not, perhaps it's better to add as suffix a timestamp instead of empty 
string?

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
Line 144:     }
Line 145: 
Line 146:     /**
Line 147:      * Suggests an alias for a disk. If the disk does not already 
have an alias, one will be generated for it. The
Line 148:      * generated alias will be formed as prefix_DiskSuffix.
the name here in javadoc is prefix_DiskSuffix , and it doesn't match the 
variable names below - they are diskPrefix and suffix. For better clarity I 
suggest to change them to match javadoc to variables. 
Also, why prefix is called diskPrefix and suffix is just called suffix without 
diskSuffix?
Line 149:      *
Line 150:      * @param disk
Line 151:      *            - The disk that (possibly) requires a new alias
Line 152:      * @param diskPrefix


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ab17da94ff7ff16852efb3a374148dacf1135f8
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Chris Morrissey <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to