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