chia7712 commented on code in PR #20332:
URL: https://github.com/apache/kafka/pull/20332#discussion_r2283709791
##########
tools/src/main/java/org/apache/kafka/tools/reassign/CompletedMoveState.java:
##########
@@ -17,36 +17,19 @@
package org.apache.kafka.tools.reassign;
-import java.util.Objects;
-
/**
* The completed replica log directory move state.
*/
-final class CompletedMoveState implements LogDirMoveState {
- public final String targetLogDir;
-
+record CompletedMoveState(String targetLogDir) implements LogDirMoveState {
/**
- * @param targetLogDir The log directory that we wanted the replica
to move to.
+ * @param targetLogDir The log directory that we wanted the replica to
move to.
*/
- public CompletedMoveState(String targetLogDir) {
- this.targetLogDir = targetLogDir;
+ CompletedMoveState {
Review Comment:
ditto
##########
tools/src/main/java/org/apache/kafka/tools/AclCommand.java:
##########
@@ -102,13 +102,7 @@ private static Properties adminConfigs(AclCommandOptions
opts) throws IOExceptio
return props;
}
- private static class AdminClientService {
-
- private final AclCommandOptions opts;
-
- AdminClientService(AclCommandOptions opts) {
- this.opts = opts;
- }
+ private record AdminClientService(AclCommandOptions opts) {
Review Comment:
Rewriting it as static helper methods seems to better than using a `record`
class. WDYT?
##########
tools/src/main/java/org/apache/kafka/tools/reassign/ActiveMoveState.java:
##########
@@ -17,44 +17,21 @@
package org.apache.kafka.tools.reassign;
-import java.util.Objects;
-
/**
* A replica log directory move state where the move is in progress.
*/
-final class ActiveMoveState implements LogDirMoveState {
- public final String currentLogDir;
-
- public final String targetLogDir;
-
- public final String futureLogDir;
-
+record ActiveMoveState(String currentLogDir, String targetLogDir, String
futureLogDir) implements LogDirMoveState {
/**
- * @param currentLogDir The current log directory.
- * @param futureLogDir The log directory that the replica is moving
to.
- * @param targetLogDir The log directory that we wanted the replica
to move to.
+ * @param currentLogDir The current log directory.
+ * @param futureLogDir The log directory that the replica is moving to.
+ * @param targetLogDir The log directory that we wanted the replica to
move to.
*/
- public ActiveMoveState(String currentLogDir, String targetLogDir, String
futureLogDir) {
- this.currentLogDir = currentLogDir;
- this.targetLogDir = targetLogDir;
- this.futureLogDir = futureLogDir;
+ ActiveMoveState {
Review Comment:
this is useless, right?
##########
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:
how about trusting the generated `toString`?
##########
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:
Based on the use cases, using `Map<TopicPartition, ProducerState>` seems
more suitable.
##########
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:
ditto
##########
tools/src/main/java/org/apache/kafka/tools/reassign/MissingReplicaMoveState.java:
##########
@@ -17,36 +17,18 @@
package org.apache.kafka.tools.reassign;
-import java.util.Objects;
-
/**
* A replica log directory move state where the source log directory is
missing.
*/
-final class MissingReplicaMoveState implements LogDirMoveState {
- public final String targetLogDir;
-
+record MissingReplicaMoveState(String targetLogDir) implements LogDirMoveState
{
/**
- * @param targetLogDir The log directory that we wanted the replica
to move to.
+ * @param targetLogDir The log directory that we wanted the replica to
move to.
*/
- public MissingReplicaMoveState(String targetLogDir) {
- this.targetLogDir = targetLogDir;
+ MissingReplicaMoveState {
Review Comment:
ditto
##########
tools/src/main/java/org/apache/kafka/tools/reassign/MissingLogDirMoveState.java:
##########
@@ -17,36 +17,18 @@
package org.apache.kafka.tools.reassign;
-import java.util.Objects;
-
/**
* A replica log directory move state where the source replica is missing.
*/
-final class MissingLogDirMoveState implements LogDirMoveState {
- public final String targetLogDir;
-
+record MissingLogDirMoveState(String targetLogDir) implements LogDirMoveState {
/**
- * @param targetLogDir The log directory that we wanted the replica
to move to.
+ * @param targetLogDir The log directory that we wanted the replica to
move to.
*/
- public MissingLogDirMoveState(String targetLogDir) {
- this.targetLogDir = targetLogDir;
+ MissingLogDirMoveState {
Review Comment:
ditto
##########
tools/src/main/java/org/apache/kafka/tools/reassign/CancelledMoveState.java:
##########
@@ -17,41 +17,21 @@
package org.apache.kafka.tools.reassign;
-import java.util.Objects;
-
/**
* A replica log directory move state where there is no move in progress, but
we did not
* reach the target log directory.
*/
-final class CancelledMoveState implements LogDirMoveState {
- public final String currentLogDir;
-
- public final String targetLogDir;
-
+record CancelledMoveState(String currentLogDir, String targetLogDir)
implements LogDirMoveState {
/**
- * @param currentLogDir The current log directory.
- * @param targetLogDir The log directory that we wanted the replica
to move to.
+ * @param currentLogDir The current log directory.
+ * @param targetLogDir The log directory that we wanted the replica to
move to.
*/
- public CancelledMoveState(String currentLogDir, String targetLogDir) {
- this.currentLogDir = currentLogDir;
- this.targetLogDir = targetLogDir;
+ CancelledMoveState {
Review Comment:
ditto
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
