Lior Vernia has posted comments on this change.

Change subject: engine: Ignore 'cfg' entries for cluster version >= 3.6
......................................................................


Patch Set 15:

(6 comments)

https://gerrit.ovirt.org/#/c/36343/15/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/BootProtocolResolver.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/BootProtocolResolver.java:

Line 30:             return;
Line 31:         }
Line 32: 
Line 33:         NetworkBootProtocol bootproto = NetworkBootProtocol.NONE;
Line 34:         String ipAddress = fetchIpAddress();
> since this one is used only once, you can inline it, since it is not releva
Done
Line 35:         if (bootProtocolDhcp()) {
Line 36:             bootproto = NetworkBootProtocol.DHCP;
Line 37:         } else if (StringUtils.isNotEmpty(ipAddress)) {
Line 38:             bootproto = NetworkBootProtocol.STATIC_IP;


Line 40: 
Line 41:         if (bootproto == NetworkBootProtocol.STATIC_IP) {
Line 42:             String gateway = fetchGateway();
Line 43:             if (StringUtils.isNotEmpty(gateway)) {
Line 44:                 VdsBrokerObjectsBuilder.setGatewayIfNecessary(iface, 
host, gateway.toString());
> the gateway type is already a String (no need toString() it)
Done
Line 45:             }
Line 46:         }
Line 47: 
Line 48:         iface.setBootProtocol(bootproto);


https://gerrit.ovirt.org/#/c/36343/15/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CfgBootProtocolResolver.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CfgBootProtocolResolver.java:

Line 12:     }
Line 13: 
Line 14:     @Override
Line 15:     protected String fetchIpAddress() {
Line 16:         return (String) entry.get("IPADDR");
> please use VdsProperties.IP_ADDRESS
Done
Line 17:     }
Line 18: 
Line 19:     @Override
Line 20:     protected String fetchGateway() {


Line 22:     }
Line 23: 
Line 24:     @Override
Line 25:     protected boolean bootProtocolDhcp() {
Line 26:         String bootProtocol = (String) this.entry.get("BOOTPROTO");
> this. prefix is redundant
Done
Line 27:         return bootProtocol != null && 
"dhcp".equalsIgnoreCase(bootProtocol);
Line 28:     }
Line 29: 


Line 23: 
Line 24:     @Override
Line 25:     protected boolean bootProtocolDhcp() {
Line 26:         String bootProtocol = (String) this.entry.get("BOOTPROTO");
Line 27:         return bootProtocol != null && 
"dhcp".equalsIgnoreCase(bootProtocol);
> you can simplified it by:
Done
Line 28:     }
Line 29: 


https://gerrit.ovirt.org/#/c/36343/15/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/NoCfgBootProtocolResolver.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/NoCfgBootProtocolResolver.java:

Line 26: 
Line 27:     @Override
Line 28:     protected boolean bootProtocolDhcp() {
Line 29:         Boolean dhcp = (Boolean) entry.get("dhcpv4");
Line 30:         return dhcp != null && dhcp;
> the 2 lines can be simplified to a single statement:
Done
Line 31:     }
Line 32: 


-- 
To view, visit https://gerrit.ovirt.org/36343
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If434111561aefab3d5d358a97331bcf3159f3484
Gerrit-PatchSet: 15
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Eliraz Levi <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: OndÅ™ej Svoboda <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to