Allon Mureinik has posted comments on this change. Change subject: core: Quota refactor - parameters ......................................................................
Patch Set 13: I would prefer that you didn't submit this (4 inline comments) giving -1 since there are some open issues IMHO that should be discussed. [although the patch generally looks ok] .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/quota/QuotaConsumptionParameter.java Line 4: import org.ovirt.engine.core.compat.Guid; Line 5: Line 6: public abstract class QuotaConsumptionParameter implements Cloneable{ Line 7: private Guid quotaGuid; Line 8: private Quota quota; I didn't understand your answer about the double members in patchset 11. let's discuss? Line 9: private QuotaAction quotaAction = QuotaAction.CONSUME; Line 10: Line 11: protected QuotaConsumptionParameter(Guid quotaGuid, Quota quota, QuotaAction quotaAction) { Line 12: this.quotaGuid = quotaGuid; .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/quota/QuotaStorageConsumptionParameter.java Line 34: this.requestedStorageGB = requestedStorageGB; Line 35: } Line 36: Line 37: @Override Line 38: public QuotaStorageConsumptionParameter clone() throws CloneNotSupportedException { In this implementation, you know the exception will never be thrown - don't declare it. Line 39: return new QuotaStorageConsumptionParameter( Line 40: getQuotaGuid(), Line 41: getQuota(), Line 42: getQuotaAction(), .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/quota/QuotaVdsGroupConsumptionParameter.java Line 45: this.requestedMemory = requestedMemory; Line 46: } Line 47: Line 48: @Override Line 49: public QuotaVdsGroupConsumptionParameter clone() throws CloneNotSupportedException { In this implementation, you know the exception will never be thrown - don't declare it. Line 50: return new QuotaVdsGroupConsumptionParameter( Line 51: getQuotaGuid(), Line 52: getQuota(), Line 53: getQuotaAction(), .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java Line 310: public static VdcActionType forValue(int value) { Line 311: return mappings.get(value); Line 312: } Line 313: Line 314: public QuotaDependency getQuotaDependency() { according to mkublin's comment and your reply on patchset 11, shouldn't this have been removed? Or did I misunderstand the comment? Line 315: return quotaDependency; Line 316: } Line 317: Line 318: /** -- To view, visit http://gerrit.ovirt.org/8775 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iebfc85569ba1aa8bd840f7239f83b7f921a4bd8e Gerrit-PatchSet: 13 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: ofri masad <oma...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com> Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com> Gerrit-Reviewer: Sharad Mishra <snmis...@linux.vnet.ibm.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: ofri masad <oma...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches