Moti Asayag has posted comments on this change.
Change subject: core: Fix uninformative quota messages (#838589)
......................................................................
Patch Set 1: (5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/quota/QuotaManager.java
Line 1: package org.ovirt.engine.core.bll.quota;
Line 2:
Line 3: import java.util.*;
The code formatter expands the * into specific import list.
That makes files comparison easier and clearer.
Perhaps your IDE requires additional tweak to support it.
Line 4: import java.util.concurrent.ConcurrentHashMap;
Line 5: import java.util.concurrent.locks.ReentrantReadWriteLock;
Line 6:
Line 7: import org.ovirt.engine.core.common.AuditLogType;
Line 36: private AuditLogableBase getLoggableQuotaStorageParams(String
quotaName,
Line 37: double storageUsagePercentage,
Line 38: double storageRequestedPercentage) {
Line 39: AuditLogableBase logable = new AuditLogableBase();
Line 40: logable.getUserName();
what's the point of having it ? it is not assigned to any variable and even if
it was assigned, its value should be null...
I think you should delete that line.
Line 41: logable.AddCustomValue("QuotaName", quotaName);
Line 42: logable.AddCustomValue("CurrentStorage",
String.valueOf((int)storageUsagePercentage));
Line 43: logable.AddCustomValue("Requested",
String.valueOf((int)storageRequestedPercentage));
Line 44:
Line 38: double storageRequestedPercentage) {
Line 39: AuditLogableBase logable = new AuditLogableBase();
Line 40: logable.getUserName();
Line 41: logable.AddCustomValue("QuotaName", quotaName);
Line 42: logable.AddCustomValue("CurrentStorage",
String.valueOf((int)storageUsagePercentage));
by casting to int you loose some of the precision. If you wish to avoid long
fraction, you can format it to print 1-2 places after the "." (e.g. 5.99)
rather than omitting it (which prints 5 for that case).
Line 43: logable.AddCustomValue("Requested",
String.valueOf((int)storageRequestedPercentage));
Line 44:
Line 45: return logable;
Line 46: }
Line 39: AuditLogableBase logable = new AuditLogableBase();
Line 40: logable.getUserName();
Line 41: logable.AddCustomValue("QuotaName", quotaName);
Line 42: logable.AddCustomValue("CurrentStorage",
String.valueOf((int)storageUsagePercentage));
Line 43: logable.AddCustomValue("Requested",
String.valueOf((int)storageRequestedPercentage));
same as above
Line 44:
Line 45: return logable;
Line 46: }
Line 47:
Line 56: AuditLogableBase logable = new AuditLogableBase();
Line 57: logable.AddCustomValue("QuotaName", quotaName);
Line 58:
Line 59: StringBuilder currentUtilization = new StringBuilder();
Line 60: currentUtilization.append(cpuOverLimit ? "vcpu:" +
String.valueOf((int)cpuCurrentPercentage) + "% " : "");
that misses the benefit of using StringBuilder which suppose to reduce the
amount of String instances.
I'd replace it with:
if (cpuOverLimit) {
currentUtilization.append("vcpu:").append(String.valueOf((int)cpuCurrentPercentage)).append("%");
}
no need for empty string, since its the default.
comment applied for both StringBuilder usages.
Line 61: currentUtilization.append(memOverLimit ? "mem:" +
String.valueOf((int)memCurrentPercentage) + "%" : "");
Line 62:
Line 63: StringBuilder request = new StringBuilder();
Line 64: request.append(cpuOverLimit ? "vcpu:" +
String.valueOf((int)cpuRequestPercentage) + "% " : "");
--
To view, visit http://gerrit.ovirt.org/7220
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7c83616b773d7471447944e6dd9adcd49d2d5ac
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: ofri masad <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-Reviewer: ofri masad <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
