Github user cristofolini commented on the pull request:
https://github.com/apache/cloudstack/pull/1310#issuecomment-171798749
Thanks for taking the time to add the descriptions and refactoring,
@nvazquez. It has helped readability quite a bit. :)
Now, in case you're interested in improving this code *even further*, may I
suggest breaking down the `postVideoCardMemoryConfigBeforeStart` method a bit
more? I like what you did separating it into two, but there are still quite a
few conditionals within each other and that makes the whole thing look
convoluted, not to mention adding up to cyclomatic complexity. Bonus points for
adding javadoc style descriptions to the new methods you create like you did to
the existing ones.
My last suggestion would be to write some test cases (unit tests) for the
methods you've created. After that, as far as I can assess, this PR will be
perfect. :)
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
