GabrielBrascher commented on a change in pull request #4205:
URL: https://github.com/apache/cloudstack/pull/4205#discussion_r654736915



##########
File path: api/src/main/java/com/cloud/network/TungstenProvider.java
##########
@@ -0,0 +1,9 @@
+package com.cloud.network;

Review comment:
       Can you please add the license header?

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -243,10 +246,12 @@
     private KVMStoragePoolManager _storagePoolMgr;
 
     private VifDriver _defaultVifDriver;
+    private VifDriver _tungstenVifDriver;

Review comment:
       Can you please remove all occurrences of underscore `_` at the beginning 
of the variable name?
   Many variables on the codebase have `_` at the beginning but it is not 
aligned with the [Java variable naming 
convention](https://docs.oracle.com/javase/tutorial/java/nutsandbolts/variables.html).
   
   According to [Java variable naming 
convention](https://docs.oracle.com/javase/tutorial/java/nutsandbolts/variables.html):
   
   > A similar convention exists for the underscore character; while it's 
technically legal to begin your variable's name with "_", this practice is 
discouraged. White space is not permitted.

##########
File path: 
engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml
##########
@@ -148,6 +148,7 @@
   <bean id="nicIpAliasDaoImpl" class="com.cloud.vm.dao.NicIpAliasDaoImpl" />
   <bean id="objectInDataStoreDaoImpl" 
class="org.apache.cloudstack.storage.db.ObjectInDataStoreDaoImpl" />
   <bean id="ovsProviderDaoImpl" 
class="com.cloud.network.dao.OvsProviderDaoImpl" />
+  <bean id="TungstenControllerDaoImpl" 
class="com.cloud.network.dao.TungstenProviderDaoImpl"/>

Review comment:
       `TungstenControllerDaoImpl` -> `tungstenControllerDaoImpl`

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -646,6 +651,9 @@ protected String getNetworkDirectDevice() {
         return _networkDirectDevice;
     }
 
+    protected String getDefaultTungstenScriptsDir() {
+        return "scripts/vm/network/tungsten";

Review comment:
       What do you think of making it a constant instead of a method?
   A method would be interesting if necessary outside of this class (as it is 
protected instead of private I assume that this is the idea).
   
   I was not able to find occurrences outside of this class but I might be 
missing something here.




-- 
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:
[email protected]


Reply via email to