Copilot commented on code in PR #13898:
URL: https://github.com/apache/skywalking/pull/13898#discussion_r3371623588
##########
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/profiling/continuous/ContinuousProfilingMutationService.java:
##########
@@ -118,32 +118,32 @@ private String
validatePolicyItem(ContinuousProfilingPolicyItemCreation item) {
switch (item.getType()) {
case PROCESS_CPU:
final int cpuPercent =
Integer.parseInt(item.getThreshold());
- if (cpuPercent < 0 || cpuPercent > 100) {
- return "the process CPU percent should in [0-100]";
+ if (cpuPercent <= 0 || cpuPercent > 100) {
+ return "the process CPU percent should in (0-100]";
}
Review Comment:
These error messages are user-facing and currently ungrammatical ("should in
..."). Since this PR is already adjusting message grammar elsewhere, consider
changing this to "should be in ..." (and optionally the more standard interval
notation "(0, 100]").
##########
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/profiling/continuous/ContinuousProfilingMutationService.java:
##########
@@ -118,32 +118,32 @@ private String
validatePolicyItem(ContinuousProfilingPolicyItemCreation item) {
switch (item.getType()) {
case PROCESS_CPU:
final int cpuPercent =
Integer.parseInt(item.getThreshold());
- if (cpuPercent < 0 || cpuPercent > 100) {
- return "the process CPU percent should in [0-100]";
+ if (cpuPercent <= 0 || cpuPercent > 100) {
+ return "the process CPU percent should in (0-100]";
}
break;
case PROCESS_THREAD_COUNT:
final int threadCount =
Integer.parseInt(item.getThreshold());
- if (threadCount < 0) {
- return "the process thread count must bigger than
zero";
+ if (threadCount <= 0) {
+ return "the process thread count must be bigger than
zero";
}
break;
case SYSTEM_LOAD:
final int systemLoad =
Integer.parseInt(item.getThreshold());
- if (systemLoad < 0) {
- return "the system load must bigger than zero";
+ if (systemLoad <= 0) {
+ return "the system load must be bigger than zero";
}
break;
case HTTP_ERROR_RATE:
final int httpErrorRate =
Integer.parseInt(item.getThreshold());
- if (httpErrorRate < 0 || httpErrorRate > 100) {
- return "the HTTP error rate should in [0-100]";
+ if (httpErrorRate <= 0 || httpErrorRate > 100) {
+ return "the HTTP error rate should in (0-100]";
}
Review Comment:
This error message is user-facing and currently ungrammatical ("should in
..."). Consider changing it to "should be in ..." for consistency with the
other grammar fixes in this PR.
##########
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/profiling/continuous/ContinuousProfilingMutationService.java:
##########
@@ -155,13 +155,13 @@ private String
validatePolicyItem(ContinuousProfilingPolicyItemCreation item) {
private String
validatePolicyItemWindows(ContinuousProfilingPolicyItemCreation item) {
if (item.getPeriod() <= 0) {
- return "period must bigger than zero";
+ return "period must be bigger than zero";
}
- if (item.getCount() < 0) {
- return "count must bigger than zero";
+ if (item.getCount() <= 0) {
+ return "count must be bigger than zero";
}
if (item.getCount() > item.getPeriod()) {
- return "count must be small than period";
+ return "count must be smaller than period";
}
Review Comment:
The validation rejects only `count > period`, so `count == period` is
currently allowed, but the error message says "count must be smaller than
period" (strictly `<`). Please either tighten the check to `>=` or adjust the
message to reflect the allowed `<=` relationship (e.g., "smaller than or equal
to period").
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
