Lior Vernia has posted comments on this change. Change subject: webadmin: Add "remove from provider" checkbox ......................................................................
Patch Set 1: (2 comments) .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/RemoveNetworksModel.java Line 51: list.add(StringFormat.format("%1$s (%2$s)", network.getName(), network.getDescription())); //$NON-NLS-1$ Line 52: } Line 53: } Line 54: Line 55: if (network.isExternal()) { It's weird that this could potentially happen multiple times. Obviously there's no harm in it, but it should only happen once, if at least one of the networks is external. I'd move it after the loop and only update a boolean value here, stating whether an external network was encountered. Line 56: getForce().setIsAvailable(true); Line 57: getForce().setEntity(true); Line 58: setForceLabel(ConstantsManager.getInstance().getConstants().removeNetworkFromProvider()); Line 59: } Line 53: } Line 54: Line 55: if (network.isExternal()) { Line 56: getForce().setIsAvailable(true); Line 57: getForce().setEntity(true); Don't think this should be the default, as this is a potentially very destructive operation. But that is up to you. Line 58: setForceLabel(ConstantsManager.getInstance().getConstants().removeNetworkFromProvider()); Line 59: } Line 60: } Line 61: setItems(list); -- To view, visit http://gerrit.ovirt.org/22615 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic39d7d2424159dd354aae3610d86b78fb8ab309e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> 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