Eliraz Levi has posted comments on this change.

Change subject: webadmin: VLanPanel refactoring
......................................................................


Patch Set 2:

(3 comments)

https://gerrit.ovirt.org/#/c/39008/2/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 1: package org.ovirt.engine.ui.webadmin.widget.host;
Line 2: 
Line 3: import com.google.gwt.user.client.ui.Widget;
> Please ask your IDE to organize imports.
Done
Line 4: import org.ovirt.engine.core.compat.StringHelper;
Line 5: import org.ovirt.engine.ui.common.widget.TogglePanel;
Line 6: import 
org.ovirt.engine.ui.uicommonweb.models.hosts.HostInterfaceLineModel;
Line 7: import org.ovirt.engine.ui.uicommonweb.models.hosts.HostVLan;


Line 93:                 lineModel.getNetworkName(),
Line 94:                 new Label(lineModel.getAddress()));
Line 95:     }
Line 96: 
Line 97:     private Grid createVlanRowColumn(Widget checkBoxWidget,
> The naming could be better, as you're not constructing a column here. Maybe
Done
Line 98:             boolean networkManagementFlag,
Line 99:             String networkName,
Line 100:             Label addressLabel) {
Line 101:         Grid row = new Grid(1, 3);


Line 96: 
Line 97:     private Grid createVlanRowColumn(Widget checkBoxWidget,
Line 98:             boolean networkManagementFlag,
Line 99:             String networkName,
Line 100:             Label addressLabel) {
> I would pass here the String instead of the Label, and only construct a Lab
Done
Line 101:         Grid row = new Grid(1, 3);
Line 102:         row.getColumnFormatter().setWidth(0, 
VLanPanel.CHECK_BOX_COLUMN_WIDTH);
Line 103:         row.getColumnFormatter().setWidth(1, 
VLanPanel.NETWORK_NAME_COLUMN_WIDTH);
Line 104:         row.getColumnFormatter().setWidth(2, 
VLanPanel.ADDRESS_COLUMN_WIDTH);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefa12d6d2876cdd1c2120a3a7667c729bd3183f9
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eliraz Levi <el...@redhat.com>
Gerrit-Reviewer: Eliraz Levi <el...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to