Yaniv Bronhaim has posted comments on this change. Change subject: ui: adding discover host integration to AddHost view ......................................................................
Patch Set 9: (5 comments) http://gerrit.ovirt.org/#/c/27623/9/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/EditHostModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/EditHostModel.java: Line 54: getCluster().setIsChangable(false); Line 55: } Line 56: Line 57: @Override Line 58: public void updateHosts() { > needed? (is it called from outside) yes, in the view.. Line 59: } Line 60: Line 61: @Override Line 62: public boolean showExternalProviderPanel() { http://gerrit.ovirt.org/#/c/27623/9/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/NewHostModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/NewHostModel.java: Line 61: if (host == null) { Line 62: host = new VDS(); Line 63: } Line 64: updateModelFromVds(host, null, false, null); Line 65: getHost().setEntity(host.getName()); > won't getName() return null in case of 'host == null' ? it'll be empty string .. its fine Line 66: getHost().setIsChangable(false); Line 67: } Line 68: Line 69: private void discoverHostName_SelectedItemChanged() { Line 106: public void onSuccess(Object model, Object result) Line 107: { Line 108: ArrayList<ExternalDiscoveredHost> hosts = (ArrayList<ExternalDiscoveredHost>) result; Line 109: ListModel externalDiscoveredHostsListModel = getExternalDiscoveredHosts(); Line 110: hosts.add(0, null); > shouldn't the first host be selected automatically? Done Line 111: externalDiscoveredHostsListModel.setItems(hosts); Line 112: externalDiscoveredHostsListModel.setIsChangable(true); Line 113: } Line 114: }; http://gerrit.ovirt.org/#/c/27623/9/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ProviderModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ProviderModel.java: Line 137: return type == ProviderType.OPENSTACK_NETWORK || type == ProviderType.OPENSTACK_IMAGE; Line 138: } Line 139: Line 140: private boolean isTypeRequiresAuthentication() { Line 141: return false; > shouldn't we still support authentication? we do, but we don't require it anymore Line 142: } Line 143: Line 144: private String getDefaultUrl(ProviderType type) { Line 145: if (type == null) { http://gerrit.ovirt.org/#/c/27623/9/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/HostPopupView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/HostPopupView.java: Line 412: @UiField(provided=true) Line 413: @Ignore Line 414: InfoIcon consoleAddressInfoIcon; Line 415: Line 416: @UiField(provided=true) > formatter ('missing whitespaces) what does it mean? Line 417: @Ignore Line 418: InfoIcon discoveredHostInfoIcon; Line 419: Line 420: @UiField(provided=true) -- To view, visit http://gerrit.ovirt.org/27623 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0faf667019760326019368f044ee16265d41a620 Gerrit-PatchSet: 9 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[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
