sjhajharia commented on code in PR #20332:
URL: https://github.com/apache/kafka/pull/20332#discussion_r2284036840


##########
tools/src/main/java/org/apache/kafka/tools/ConnectPluginPath.java:
##########
@@ -196,22 +196,8 @@ enum Command {
         LIST, SYNC_MANIFESTS
     }
 
-    private static class Config {
-        private final Command command;
-        private final Set<Path> locations;
-        private final boolean dryRun;
-        private final boolean keepNotFound;
-        private final PrintStream out;
-        private final PrintStream err;
-
-        private Config(Command command, Set<Path> locations, boolean dryRun, 
boolean keepNotFound, PrintStream out, PrintStream err) {
-            this.command = command;
-            this.locations = locations;
-            this.dryRun = dryRun;
-            this.keepNotFound = keepNotFound;
-            this.out = out;
-            this.err = err;
-        }
+    private record Config(Command command, Set<Path> locations, boolean 
dryRun, boolean keepNotFound, PrintStream out,
+                          PrintStream err) {
 
         @Override
         public String toString() {

Review Comment:
   The auto generated one will also have the `out` and `err` params which the 
record takes in. Hence I prefered keeping this custom one.



##########
tools/src/main/java/org/apache/kafka/tools/TransactionsCommand.java:
##########
@@ -848,17 +847,7 @@ private List<OpenTransaction> 
collectCandidateOpenTransactions(
             return candidateTransactions;
         }
 
-        private static class OpenTransaction {
-            private final TopicPartition topicPartition;
-            private final ProducerState producerState;
-
-            private OpenTransaction(
-                TopicPartition topicPartition,
-                ProducerState producerState
-            ) {
-                this.topicPartition = topicPartition;
-                this.producerState = producerState;
-            }
+        private record OpenTransaction(TopicPartition topicPartition, 
ProducerState producerState) {

Review Comment:
   Sorry, but I dont cpmpletely follow here. Do we want to change the way 
`OpenTransaction` is defined? 
   If so, can we create a ticket and deal with the same separately as it may 
cause some additional changes in this file.



##########
tools/src/test/java/org/apache/kafka/tools/ConnectPluginPathTest.java:
##########
@@ -444,13 +444,7 @@ private enum PluginLocationType {
         MULTI_JAR
     }
 
-    private static class PluginLocation {
-        private final Path path;
-
-        private PluginLocation(Path path) {
-            this.path = path;
-        }
-
+    private record PluginLocation(Path path) {
         @Override
         public String toString() {

Review Comment:
   I don't think we can get rid of this toString() method. 
   The default Java toString would create a value like 
"PluginLocation[path=...]", but the expectation here is to just have the path 
and not the name of the class.



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