cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GabrielBrascher <...@git.apache.org>
Subject [GitHub] cloudstack pull request: [4.7] vmware: improve support for disk co...
Date Fri, 29 Jan 2016 00:27:32 GMT
Github user GabrielBrascher commented on the pull request:

    https://github.com/apache/cloudstack/pull/1365#issuecomment-176495485
  
    @bhaisaab Could you please do the following changes?
    
    At **VirtualMachineVolumeChainInfo** class:
    **1 -** create Javadoc blocks (would be nice to document all the class, but I think that
at least the class and the "getControllerFromDeviceBusName" method deserves a documentation);
    **2 -** simplify the **if** conditional at line **43**. Test with "( (StringUtils.isEmpty)
|| (!this.diskDeviceBusName.contains(":")) )" condition. The StringUtils.isEmpty method already
checks if the String is empty("") or null (https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/StringUtils.html).
    **3 -** create test for the getControllerFromDeviceBusName method.
    
    At **UserVmManagerImpl** class:
    **1 -** document the "persistDeviceBusInfo(UserVmVO vm, String rootDiskController)" method
(lines **5458-5466**).
    **2 -** use the "StringUtils.isEmpty()" with the condition at line **5460**, instead of
"(existingVmRootDiskController==null) || (existingVmRootDiskController.isEmpty())".
    **3 -** create test for "persistDeviceBusInfo(UserVmVO vm, String rootDiskController)"
method.
    
    **VolumeApiServiceImpl**:
    **1 -** Remove “**_**” from variables names (lines **200**, **252**, **267**): private
variables with “**_**” at the beginning is common in C++ but not in Java.
    
    Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message