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

Reply via email to