ableegoldman commented on a change in pull request #11584:
URL: https://github.com/apache/kafka/pull/11584#discussion_r800368829



##########
File path: 
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java
##########
@@ -1516,4 +1527,20 @@ RebalanceProtocol getProtocol() {
     boolean poll(Timer timer) {
         return poll(timer, true);
     }
+
+

Review comment:
       nit: extra lines

##########
File path: 
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java
##########
@@ -1516,4 +1527,20 @@ RebalanceProtocol getProtocol() {
     boolean poll(Timer timer) {
         return poll(timer, true);
     }
+
+
+
+    final static class TopicPartitionComparator implements 
Comparator<TopicPartition>, Serializable {

Review comment:
       nit: seems a bit weird to have this in the `ConsumerCoordinator`, would 
make more sense in a utility class or something related to TopicPartitions -- 
obviously the actual TopicPartition class would be ideal but unfortunately that 
would require a KIP, not sure what the second best choice would be but maybe 
`Topic` or `Utils` or even `CollectionUtils` (under the clients/common package)




-- 
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


Reply via email to