GabrielBrascher commented on a change in pull request #3186: Add possibility to set KVM MTU size for all NIC URL: https://github.com/apache/cloudstack/pull/3186#discussion_r346473236
########## File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java ########## @@ -247,6 +262,10 @@ @Inject ConfigurationManager _configMgr; @Inject + ClusterDao _clusterDao; + @Inject + ClusterDetailsDao _clusterDetailsDao; Review comment: I really don't want to sound picky or negative here, just adding a few minor comments :-) - those variables could be set to private; - additionally, could you please remove the underscore ('_') at the beginning of variables names? I know that we see this a lot around the CloudStack codebase but this is not really the best approach. While it's technically legal to begin your variable's name with "_" or "$", this practice is discouraged by the Java variable naming convention. ---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services