Re: [PR] KAFKA-200135 Add javadoc to org.apache.kafka.clients.admin module(1/N) [kafka]

["kirktrue (via GitHub)" <gi...@apache.org>]

kirktrue commented on code in PR #21484:
URL: https://github.com/apache/kafka/pull/21484#discussion_r2908580145


##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -1250,6 +1250,8 @@ default ListPartitionReassignmentsResult listPartitionReassignments(ListPartitio
     }
 
     /**
+     * List all the current partition reassignments
+     * 
      * @param partitions the partitions we want to get reassignment for, or an empty optional if we want to get the reassignments for all partitions in the cluster

Review Comment:
   We're capitalizing the start of the sentences for the parameters. We should be consistent. Also we may want to spit this line in two.



##########
clients/src/main/java/org/apache/kafka/clients/admin/Admin.java:
##########
@@ -1250,6 +1250,8 @@ default ListPartitionReassignmentsResult listPartitionReassignments(ListPartitio
     }
 
     /**
+     * List all the current partition reassignments

Review Comment:
   Seems like we want to end this sentence with a period ('.').



##########
clients/src/main/java/org/apache/kafka/clients/admin/AdminClientConfig.java:
##########
@@ -308,14 +354,25 @@ protected AdminClientConfig(Map<?, ?> props, boolean doLog) {
         super(CONFIG, props, doLog);
     }
 
+    /**
+     * Get the set of configuration names supported by this configuration class.
+     * @return the set of configuration names.

Review Comment:
   Inconsistent capitalization.
   
   ```suggestion
        * @return The set of configuration names.
   ```



##########
clients/src/main/java/org/apache/kafka/clients/admin/AdminClientConfig.java:
##########
@@ -39,7 +39,14 @@
 import static org.apache.kafka.common.config.ConfigDef.ValidString.in;
 
 /**
- * The AdminClient configuration class, which also contains constants for configuration entry names.
+ * Configuration for the {@link Admin} client. This class defines all configuration properties
+ * accepted by the admin client and provides constants for their names.
+ * <p>
+ * At minimum, {@link #BOOTSTRAP_SERVERS_CONFIG} must be set to connect to brokers, or

Review Comment:
   ```suggestion
    * At minimum, <em>either</em> {@link #BOOTSTRAP_SERVERS_CONFIG} must be set to connect to brokers, or
   ```



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org