[GitHub] [cloudstack] kioie commented on pull request #4010: New API endpoint to update size of Pod Management IP Range.

[GitBox <gi...@apache.org>]

kioie commented on pull request #4010:
URL: https://github.com/apache/cloudstack/pull/4010#issuecomment-654101067


   > This is an interesting addition, thanks for the PR @kioie 👍.
   > 
   > I have a few points that I would like to be checked:
   > 
   >     1. check code style, a few code pieces are not aligned with CloudStack code conventions
   > 
   >     2. I think that this code can be even better if breaking the update method into smaller methods
   > 
   >     3. if possible, can you please add JUnit test cases on [ConfigurationManagerTest.java](https://github.com/apache/cloudstack/blob/master/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java)?
   > 
   > 
   > Regarding 2: small and concise methods, with self-explaining names and Javadoc, helps when reviewing, debugging, and maintaining the code base; additionally, with smaller methods, it is easy to create JUnit test case methods.
   
   Thanks.
   Regarding the tests cases, any suggestions on which methods I should consider?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org