Greg Padgett has posted comments on this change. Change subject: webadmin: Allow selection of None for Cloud-Init network boot protocol ......................................................................
Patch Set 2: (3 comments) @Lior - Thanks for having a look, some replies in-line. The reason I had the mapping and constants for the enum values was to make the display a little more refined, e.g. "Static IP" (i18n-capable, too) rather than "STATIC_IP". Your suggestion definitely would make the code simpler, which I can appreciate, but do you think it's worth adding (imo) a "rough edge"? Since the work is already done, my opinion is to leave it it. 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 Will change 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 Will change 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 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().getSel Okay, interesting. I'll have to make sure that cloud-init accepts that, and that the validator accepts "none" with and without an address. (So it seems "static" and "none" are pretty much the same, but different enough for this bz to exist :)) Line 611: || model.getNetworkBootProtocolList().getSelectedItem().getValue() == NetworkBootProtocol.STATIC_IP); Line 612: } Line 613: }); Line 614: -- 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
