Lior Vernia has posted comments on this change.

Change subject: webadmin: adding network-out-of-sync to SubTabNetworkHost
......................................................................


Patch Set 5:

(2 comments)

https://gerrit.ovirt.org/#/c/38291/5/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/host/VLanPanel.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/host/VLanPanel.java:

Line 71
Line 72
Line 73
Line 74
Line 75
Why did you need to remove this line?


Line 77:         Grid row =
Line 78:                 createVlanRowColumn(checkboxPanel,
Line 79:                         hostVLan.getInterface().getIsManagement(),
Line 80:                         hostVLan.getNetworkName(),
Line 81:                         
createSyncPanel(hostVLan.getInterface().getNetworkImplementationDetails().isInSync()),
It's strange to me that here you don't have to consider the null implementation 
details. Isn't it possible to get to such a situation if a VLAN device is 
configured on the host without a network over it? In that case you'd still need 
this to not throw a null pointer exception.

Please verify, and if this situation can be reached, the code dealing with null 
implementation details should be common to both flows (say be extracting it to 
createSyncPanel()).
Line 82:                         new Label(hostVLan.getAddress()));
Line 83: 
Line 84:         Style gridStyle = row.getElement().getStyle();
Line 85:         gridStyle.setBorderColor("white"); //$NON-NLS-1$


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id200f9c135f1097572feac300e255a438cfb295a
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eliraz Levi <[email protected]>
Gerrit-Reviewer: Eliraz Levi <[email protected]>
Gerrit-Reviewer: Lior Vernia <[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