Lior Vernia has posted comments on this change. Change subject: webadmin: Allow selection of None for Cloud-Init network boot protocol ......................................................................
Patch Set 2: (11 comments) A couple of comments to simplify the code. http://gerrit.ovirt.org/#/c/31051/2/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java: Line 228: Line 229: @DefaultStringValue("Select network above") Line 230: String cloudInitNetworkSelectLabel(); Line 231: Line 232: @DefaultStringValue("Address Type") "Boot Protocol" is a common term in networking, I would use that instead of "Address Type". Line 233: String cloudInitNetworkBootProtocolLabel(); Line 234: Line 235: @DefaultStringValue("IP Address") Line 236: String cloudInitNetworkIpAddressLabel(); Line 309: Line 310: @DefaultStringValue("Enter the name of a network interface, e.g. \"eth0\"") Line 311: String cloudInitNetworkToolTip(); Line 312: Line 313: @DefaultStringValue("Select no address, DHCP, or Static IP for the selected network interface") Same comment here, "Choose boot protocol for the selected network interface" should be fine. Line 314: String cloudInitNetworkBootProtocolToolTip(); Line 315: Line 316: @DefaultStringValue("Enter the IP address for the selected network interface") Line 317: String cloudInitNetworkIpAddressToolTip(); http://gerrit.ovirt.org/#/c/31051/2/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmInitWidget.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmInitWidget.java: Line 319: Line 320: @UiField(provided = true) Line 321: @Path(value = "networkBootProtocolList.selectedItem") Line 322: @WithElementId Line 323: ListModelListBoxEditor<Map.Entry<String, NetworkBootProtocol>> networkBootProtocolEditor; This can more simply be ListModelListBoxEditor<NetworkBootProtocol>... Line 324: Line 325: @UiField Line 326: @Path(value = "networkIpAddress.entity") Line 327: @WithElementId Line 402: return object.getValue(); Line 403: } Line 404: }); Line 405: Line 406: networkBootProtocolEditor = new ListModelListBoxEditor<Map.Entry<String, NetworkBootProtocol>>(new NullSafeRenderer<Map.Entry<String, NetworkBootProtocol>>() { ...and then here you could use EnumRenderer. (and some more code is made redundant in the model, see comments there) Line 407: @Override Line 408: protected String renderNullSafe(Map.Entry<String, NetworkBootProtocol> object) { Line 409: return object.getKey(); Line 410: } Line 606: Line 607: model.getNetworkBootProtocolList().getSelectedItemChangedEvent().addListener(new IEventListener() { Line 608: @Override Line 609: public void eventRaised(Event ev, Object sender, EventArgs args) { Line 610: setNetworkStaticDetailsStyle(model.getNetworkBootProtocolList().getSelectedItem() == null The entire condition should probably be getNetworkBootProtocolList().getSelectedItem() != NetworkBootProtocol.DHCP. Strangely enough it's valid in Linux to specify boot protocol as NONE and supply an IP address (NONE nowadays is pretty much the same as STATIC). Line 611: || model.getNetworkBootProtocolList().getSelectedItem().getValue() == NetworkBootProtocol.STATIC_IP); Line 612: } Line 613: }); Line 614: http://gerrit.ovirt.org/#/c/31051/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmInitModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmInitModel.java: Line 311: private void setRemoveNetworkCommand(UICommand value) { Line 312: removeNetworkCommand = value; Line 313: } Line 314: Line 315: private ListModel<Map.Entry<String, NetworkBootProtocol>> privateNetworkBootProtocolList; This should be changed to ListModel<NetworkBootProtocol> corresponding to the change in the view... Line 316: public ListModel<Map.Entry<String, NetworkBootProtocol>> getNetworkBootProtocolList() { Line 317: return privateNetworkBootProtocolList; Line 318: } Line 319: private void setNetworkBootProtocolList(ListModel<Map.Entry<String, NetworkBootProtocol>> value) { Line 434: private static final String base64Message; Line 435: private static final String base64Regex; Line 436: private static final String networkNoneText; Line 437: private static final String networkDhcpText; Line 438: private static final String networkStaticIpText; ...then these members wouldn't be necessary... Line 439: Line 440: private SortedMap<String, VmInitNetwork> networkMap; Line 441: private Set<String> networkStartOnBoot; Line 442: private String lastSelectedNetworkName; Line 576: Map<String, NetworkBootProtocol> networkBootProtocols = new HashMap<String, NetworkBootProtocol>(); Line 577: networkBootProtocols.put(networkNoneText, NetworkBootProtocol.NONE); Line 578: networkBootProtocols.put(networkDhcpText, NetworkBootProtocol.DHCP); Line 579: networkBootProtocols.put(networkStaticIpText, NetworkBootProtocol.STATIC_IP); Line 580: getNetworkBootProtocolList().setItems(networkBootProtocols.entrySet()); ...and this whole thing could be replaced with getNetworkBootProtocolList().setItems(NetworkBootProtocol.values())... Line 581: getNetworkBootProtocolList().setSelectedItem(Linq.firstOrDefault(networkBootProtocols.entrySet(), Line 582: new IPredicate<Map.Entry<String, NetworkBootProtocol>>() { Line 583: @Override Line 584: public boolean match(Map.Entry<String, NetworkBootProtocol> item) { Line 577: networkBootProtocols.put(networkNoneText, NetworkBootProtocol.NONE); Line 578: networkBootProtocols.put(networkDhcpText, NetworkBootProtocol.DHCP); Line 579: networkBootProtocols.put(networkStaticIpText, NetworkBootProtocol.STATIC_IP); Line 580: getNetworkBootProtocolList().setItems(networkBootProtocols.entrySet()); Line 581: getNetworkBootProtocolList().setSelectedItem(Linq.firstOrDefault(networkBootProtocols.entrySet(), ...and here you could simply set selected item to NetworkBootProtocol.NONE. Line 582: new IPredicate<Map.Entry<String, NetworkBootProtocol>>() { Line 583: @Override Line 584: public boolean match(Map.Entry<String, NetworkBootProtocol> item) { Line 585: return item.getValue() == NetworkBootProtocol.NONE; Line 1063: } Line 1064: Line 1065: final NetworkBootProtocol bootProtocol = ((obj == null || obj.getBootProtocol() == null) Line 1066: ? NetworkBootProtocol.NONE : obj.getBootProtocol()); Line 1067: getNetworkBootProtocolList().setSelectedItem(Linq.firstOrDefault(getNetworkBootProtocolList().getItems(), This could be simplified as above. Line 1068: new IPredicate<Map.Entry<String, NetworkBootProtocol>>() { Line 1069: @Override Line 1070: public boolean match(Map.Entry<String, NetworkBootProtocol> item) { Line 1071: return item.getValue() == bootProtocol; http://gerrit.ovirt.org/#/c/31051/2/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java File frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java: Line 2166: @DefaultStringValue("DHCP") Line 2167: String cloudInitNetworkTypeDHCP(); Line 2168: Line 2169: @DefaultStringValue("Static IP") Line 2170: String cloudInitNetworkTypeStaticIP(); These three values could be removed as well, if EnumRenderer is used. Line 2171: Line 2172: @DefaultStringValue("VM Interface Profile") Line 2173: String vnicProfileTitle(); Line 2174: -- To view, visit http://gerrit.ovirt.org/31051 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic51a76e968922fb62a07ccaced969c8b580de083 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Greg Padgett <[email protected]> Gerrit-Reviewer: Greg Padgett <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[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
