mimaison commented on code in PR #16065:
URL: https://github.com/apache/kafka/pull/16065#discussion_r1626203185
##########
metadata/src/test/java/org/apache/kafka/controller/ReplicationControlManagerTest.java:
##########
@@ -2664,7 +2664,7 @@ public void testKRaftClusterDescriber() {
ctx.registerBrokersWithDirs(
0, Collections.emptyList(),
1, Collections.emptyList(),
- 2, asList(Uuid.fromString("ozwqsVMFSNiYQUPSJA3j0w")),
+ 2, singletonList(Uuid.fromString("ozwqsVMFSNiYQUPSJA3j0w")),
Review Comment:
Should we keep `asList()` here so it's similar to the line below?
##########
metadata/src/test/java/org/apache/kafka/image/ImageDowngradeTest.java:
##########
@@ -103,7 +103,7 @@ public void testPremodernVersion() {
@Test
public void testPreControlledShutdownStateVersion() {
writeWithExpectedLosses(MetadataVersion.IBP_3_3_IV2,
- Arrays.asList(
+ Collections.singletonList(
Review Comment:
Can we use the same alignment as in the other methods?
##########
metadata/src/test/java/org/apache/kafka/image/loader/MetadataLoaderTest.java:
##########
@@ -249,17 +249,17 @@ public void testPublisherCannotBeInstalledMoreThanOnce(
setFaultHandler(faultHandler).
setHighWaterMarkAccessor(() -> OptionalLong.of(0L)).
build()) {
- loader.installPublishers(asList(publisher)).get();
+
loader.installPublishers(Collections.singletonList(publisher)).get();
Review Comment:
I wonder if we should import static `singletonList()` in this file because
`Collections.singletonList()` is adding a lot of noise compared to the much
shorter `asList()`
##########
metadata/src/test/java/org/apache/kafka/controller/QuorumControllerTestEnv.java:
##########
@@ -49,7 +49,7 @@ public static class Builder {
private Consumer<QuorumController.Builder>
controllerBuilderInitializer = __ -> { };
private OptionalLong sessionTimeoutMillis = OptionalLong.empty();
private OptionalLong leaderImbalanceCheckIntervalNs =
OptionalLong.empty();
- private boolean eligibleLeaderReplicasEnabled = false;
+ private final boolean eligibleLeaderReplicasEnabled = false;
Review Comment:
This field is not used, should we delete it?
##########
metadata/src/test/java/org/apache/kafka/image/writer/RaftSnapshotWriterTest.java:
##########
@@ -44,7 +45,7 @@ public void testFreezeAndClose() {
assertTrue(snapshotWriter.isClosed());
assertEquals(Arrays.asList(
Arrays.asList(testRecord(0), testRecord(1)),
- Arrays.asList(testRecord(2))), snapshotWriter.batches());
+ Collections.singletonList(testRecord(2))),
snapshotWriter.batches());
Review Comment:
I'd keep `Arrays.asList()` here so it's similar to the line above.
--
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]
