Moti Asayag has posted comments on this change.
Change subject: webadmin: add Network as main tab (phase 1)
......................................................................
Patch Set 10: (15 inline comments)
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/networks/NetworkGeneralModel.java
Line 5: import org.ovirt.engine.core.compat.PropertyChangedEventArgs;
Line 6: import org.ovirt.engine.core.compat.StringHelper;
Line 7: import org.ovirt.engine.ui.uicommonweb.models.EntityModel;
Line 8: import org.ovirt.engine.ui.uicompat.ConstantsManager;
Line 9:
could you consider organizing the file in the following order:
* members
* c'tor
* methods
* getters/setters
Line 10: public class NetworkGeneralModel extends EntityModel
Line 11: {
Line 12: private String privateName;
Line 13:
Line 8: import org.ovirt.engine.ui.uicompat.ConstantsManager;
Line 9:
Line 10: public class NetworkGeneralModel extends EntityModel
Line 11: {
Line 12: private String privateName;
s/privateName/name or networkName
Line 13:
Line 14: public String getName()
Line 15: {
Line 16: return privateName;
Line 24: OnPropertyChanged(new PropertyChangedEventArgs("Name"));
//$NON-NLS-1$
Line 25: }
Line 26: }
Line 27:
Line 28: private Boolean privateVm;
same same as above
Line 29:
Line 30: public Boolean getVm()
Line 31: {
Line 32: return privateVm;
Line 40: OnPropertyChanged(new PropertyChangedEventArgs("Vm"));
//$NON-NLS-1$
Line 41: }
Line 42: }
Line 43:
Line 44: private Integer privateVlan;
same as above
Line 45:
Line 46: public Integer getVlan()
Line 47: {
Line 48: return privateVlan;
Line 60: OnPropertyChanged(new PropertyChangedEventArgs("Vlan"));
//$NON-NLS-1$
Line 61: }
Line 62: }
Line 63:
Line 64: private Integer privateMtu;
same as above
Line 65:
Line 66: public Integer getMtu()
Line 67: {
Line 68: return privateMtu;
Line 76: OnPropertyChanged(new PropertyChangedEventArgs("Mtu"));
//$NON-NLS-1$
Line 77: }
Line 78: }
Line 79:
Line 80: private String privateDescription;
same as above
Line 81:
Line 82: public String getDescription()
Line 83: {
Line 84: return privateDescription;
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/networks/NetworkListModel.java
Line 21: import org.ovirt.engine.ui.uicommonweb.models.SystemTreeItemModel;
Line 22: import
org.ovirt.engine.ui.uicommonweb.models.configure.PermissionListModel;
Line 23: import org.ovirt.engine.ui.uicompat.ConstantsManager;
Line 24:
Line 25: public class NetworkListModel extends ListWithDetailsModel implements
ISupportSystemTreeContext
same comments as given on
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/networks/NetworkGeneralModel.java
Line 26: {
Line 27: private static String ENGINE_NETWORK;
Line 28:
Line 29: private UICommand privateNewCommand;
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/networks/NetworkTemplateListModel.java
Line 64: }
Line 65:
Line 66: super.SyncSearch();
Line 67:
Line 68: AsyncQuery _asyncQuery = new AsyncQuery();
s/_asyncQuery/asyncQuery
Line 69: _asyncQuery.setModel(this);
Line 70: // _asyncQuery.asyncCallback = new INewAsyncCallback() {
Line 71: // @Override
Line 72: // public void OnSuccess(Object model, Object ReturnValue)
Line 65:
Line 66: super.SyncSearch();
Line 67:
Line 68: AsyncQuery _asyncQuery = new AsyncQuery();
Line 69: _asyncQuery.setModel(this);
don't leave comment code in the source file
Line 70: // _asyncQuery.asyncCallback = new INewAsyncCallback() {
Line 71: // @Override
Line 72: // public void OnSuccess(Object model, Object ReturnValue)
Line 73: // {
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SystemTreeModel.java
Line 125: {
Line 126: privateClusterMap = value;
Line 127: }
Line 128:
Line 129: private HashMap<Guid, List<Network>> privateNetworkMap;
private as a variable name is redundant and doesn't comply with java naming
convention. please replace with networkMap.
Line 130:
Line 131: public HashMap<Guid, List<Network>> getNetworkMap()
Line 132: {
Line 133: return privateNetworkMap;
Line 286:
Line 287:
systemTreeModel.setNetworkMap(new HashMap<Guid, List<Network>>());
Line 288:
Line 289:
List<VdcQueryReturnValue> returnValueList = result.getReturnValues();
Line 290: List<Network>
dcNetworkList = null;
it seems that it is not required to initialize the dcNetworkList and the dcId
as being overridden before being used.
Line 291: Guid dcId = null;
Line 292:
Line 293: for (int i = 0; i <
returnValueList.size(); i++)
Line 294: {
Line 442: networksItem.setEntity(getDataCenters().get(count));
Line 443: dataCenterItem.getChildren().add(networksItem);
Line 444:
Line 445: List<Network> dcNetworks =
getNetworkMap().get(getDataCenters().get(count).getId());
Line 446: if (dcNetworks != null && dcNetworks.size() > 0)
"dcNetworks.size() > 0" can be replaced with !dcNetworks.isEmpty()
Line 447: {
Line 448: for (Network network : dcNetworks)
Line 449: {
Line 450: SystemTreeItemModel networkItem = new
SystemTreeItemModel();
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java
Line 1478:
Line 1479: @DefaultStringValue("Data Center")
Line 1480: String dcNetwork();
Line 1481:
Line 1482: @DefaultStringValue("VLAN tagging")
shouldn't it be 'VLAN tag' ?
Line 1483: String vlanNetwork();
Line 1484:
Line 1485: @DefaultStringValue("MTU")
Line 1486: String mtuNetwork();
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/network/NetworkSubTabPanelPresenter.java
Line 21:
Line 22: public interface ViewDef extends TabView {
Line 23: }
Line 24:
Line 25: @RequestTabs
please define the static members at the begin of the file
Line 26: public static final Type<RequestTabsHandler> TYPE_RequestTabs =
new Type<RequestTabsHandler>();
Line 27:
Line 28: @ContentSlot
Line 29: public static final Type<RevealContentHandler<?>>
TYPE_SetTabContent = new Type<RevealContentHandler<?>>();
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/network/SubTabNetworkGeneralView.ui.xml
Line 2: <!DOCTYPE ui:UiBinder SYSTEM "http://dl.google.com/gwt/DTD/xhtml.ent";>
Line 3: <ui:UiBinder xmlns:ui="urn:ui:com.google.gwt.uibinder"
Line 4: xmlns:g="urn:import:com.google.gwt.user.client.ui"
xmlns:e="urn:import:com.google.gwt.editor.ui.client"
Line 5: xmlns:f="urn:import:org.ovirt.engine.ui.common.widget.form">
Line 6:
please remove trailing white spaces
Line 7: <ui:style>
Line 8: .formPanel {
Line 9: padding-top: 10px;
Line 10: }
--
To view, visit http://gerrit.ovirt.org/8454
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d641f5c7c4386f406bbc834c549ac7dd12d43cf
Gerrit-PatchSet: 10
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Muli Salem <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
