dimitarndimitrov commented on code in PR #13432:
URL: https://github.com/apache/kafka/pull/13432#discussion_r1169819833
##########
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##########
@@ -4579,7 +4374,7 @@ void maybeRetry(long currentTimeMs, Throwable throwable) {
};
}
- private long getOffsetFromOffsetSpec(OffsetSpec offsetSpec) {
+ private static long getOffsetFromSpec(OffsetSpec offsetSpec) {
Review Comment:
Preface: I am totally fine with reverting these if you prefer so.
Having said that, can I use this as an opportunity to educate myself on
preferred style in AK? Let me know if it's preferable to avoid minor code
changes like these if the justifications for the change are mostly stylistic,
like mine below:
I added the `static` modifier as it indicates that the corresponding method
doesn't read or mutate instance state and in this specific case (where class
state is also not affected) it's a relatively simple utility method. For a
class with as many other methods as `KafkaAdminClient` this seemed useful.
I shortened the name as the current one seemed tautological and because the
sole usage of the method is in a bit of a wordy expression to begin with:
```
Map<TopicPartition, Long> offsetQueriesByPartition =
topicPartitionOffsets.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, e ->
getOffsetFromSpec(e.getValue())));
```
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
