Daniel Erez has posted comments on this change.
Change subject: engine: remove disabled external scheduler plugin
......................................................................
Patch Set 2:
(8 comments)
+1 for frontend
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Linq.java
Line 1267: if (pu1.getPolicyUnitType() != pu2.getPolicyUnitType()) {
Line 1268: if
(pu1.getPolicyUnitType().equals(PolicyUnitType.Filter)) {
Line 1269: return -1;
Line 1270: }
Line 1271: if
(pu2.getPolicyUnitType().equals(PolicyUnitType.Filter)) {
why not combine both this conditions into one?
Line 1272: return 1;
Line 1273: }
Line 1274: if
(pu1.getPolicyUnitType().equals(PolicyUnitType.LoadBalancing)) {
Line 1275: return 1;
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/configure/scheduling/ClusterPolicyListModel.java
Line 263: policyUnitModel.setHashName("manage_policy_units");
//$NON-NLS-1$
Line 264:
Line 265: policyUnitModel.setPolicyUnits(new ListModel());
Line 266: policyUnitModel.getPolicyUnits().setItems(sort(policyUnits));
Line 267: policyUnitModel.setRefreshPolicyUnits(this);
consider using events instead of sending the entire ListModel
Line 268:
Line 269: UICommand command = new UICommand("Cancel", this);
//$NON-NLS-1$
Line 270:
command.setTitle(ConstantsManager.getInstance().getConstants().close());
Line 271: command.setIsCancel(true);
Line 273:
Line 274: setWindow(policyUnitModel);
Line 275: }
Line 276:
Line 277: private ArrayList<PolicyUnit> sort(ArrayList<PolicyUnit>
policyUnits) {
probably not for this patch, but can we externalize/generalize this fluent api
for sorting?
Line 278: Collections.sort(policyUnits, new
Linq.PolicyUnitComparator());
Line 279: return policyUnits;
Line 280: }
Line 281:
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/configure/scheduling/ManagePolicyUnitModel.java
Line 36:
Line 37: @Override
Line 38: public void executed(FrontendActionAsyncResult
result) {
Line 39: if (result.getReturnValue() != null &&
result.getReturnValue().getSucceeded()) {
Line 40:
getRefreshPolicyUnits().refreshPolicyUnits();
consider sending an event to the ListModel instead of passing the instance and
introducing an interface.
Line 41: }
Line 42: }
Line 43: });
Line 44: }
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationResources.java
Line 93:
Line 94: @Source("images/Lock.png")
Line 95: ImageResource lockImage();
Line 96:
Line 97: @Source("images/ico_external.png")
can you rename the file name to something like: "ico_external_policy". Just for
giving an indication that it's related to external policy units...
Line 98: ImageResource exteranlPolicyUnitImage();
Line 99:
Line 100: @Source("images/logo.png")
Line 101: ImageResource logoImage();
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/scheduling/ManagePolicyUnitPopupView.java
Line 67:
Line 68: private void initTable(final ApplicationResources resources,
Line 69: final ApplicationConstants constants,
Line 70: ApplicationMessages messages) {
Line 71: policyUnitTable = new ListModelObjectCellTable<PolicyUnit,
ListModel>();
shouldn't this table support resizable columns?
Line 72: policyUnitTableContainer.add(policyUnitTable);
Line 73:
Line 74: policyUnitTable.addColumn(new
WebAdminImageResourceColumn<PolicyUnit>() {
Line 75: @Override
Line 101: public String getValue(PolicyUnit object) {
Line 102: if (!object.isEnabled()) {
Line 103: return constants.disabledPolicyUnit();
Line 104: }
Line 105: return ""; //$NON-NLS-1$
use constants.empty()
Line 106: }
Line 107: });
Line 108:
Line 109: Column<PolicyUnit, String> removeButtonColumn = new
Column<PolicyUnit, String>(new NullableButtonCell()) {
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/scheduling/ManagePolicyUnitPopupView.ui.xml
Line 3: <ui:UiBinder xmlns:ui="urn:ui:com.google.gwt.uibinder"
Line 4: xmlns:g="urn:import:com.google.gwt.user.client.ui"
xmlns:d="urn:import:org.ovirt.engine.ui.common.widget.dialog"
Line 5: xmlns:e="urn:import:org.ovirt.engine.ui.common.widget.editor"
xmlns:we="urn:import:org.ovirt.engine.ui.webadmin.widget.editor">
Line 6:
Line 7: <ui:style>
format: please use spaces instead of tabs
Line 8: .containerStyle {
Line 9: height: 350px;
Line 10: }
Line 11: </ui:style>
--
To view, visit http://gerrit.ovirt.org/19796
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia03da19368b75b125aa3ea144b36016feddcb08c
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@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
