adoroszlai commented on code in PR #6332:
URL: https://github.com/apache/ozone/pull/6332#discussion_r1514028323
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/ContainerClientMetrics.java:
##########
@@ -112,27 +112,27 @@ public void recordWriteChunk(Pipeline pipeline, long
chunkSizeBytes) {
}
@VisibleForTesting
- public MutableCounterLong getTotalWriteChunkBytes() {
+ MutableCounterLong getTotalWriteChunkBytes() {
return totalWriteChunkBytes;
}
@VisibleForTesting
Review Comment:
I also considered removing the annotation. Here's why I kept it (so far):
Javadoc of `@VisibleForTesting` says it should not be used on `public` or
`protected` members, since that indicates bad design. By that logic it should
only be applied to members with package-private (default) visibility. So
removing it when tests are in the same package means we would never use the
annotation.
On the other hand, I'm not sure if it's worth sprinkling all over the code
for trivial accessors. Maybe we should limit it to methods with actual logic.
Such methods exist in their own right for the class itself, only visibility is
affected by usage in tests.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
