dajac commented on code in PR #14056:
URL: https://github.com/apache/kafka/pull/14056#discussion_r1277524963
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java:
##########
@@ -843,15 +886,19 @@ public JoinGroupResponseData
setupGroupWithPendingMember(GenericGroup group) thr
// Start the join for the second member
CompletableFuture<JoinGroupResponseData> followerJoinFuture = new
CompletableFuture<>();
CoordinatorResult<Void, Record> result = sendGenericGroupJoin(
- joinRequest.setMemberId(UNKNOWN_MEMBER_ID),
+ joinRequest
+ .setMemberId(UNKNOWN_MEMBER_ID),
followerJoinFuture
);
assertTrue(result.records().isEmpty());
assertFalse(followerJoinFuture.isDone());
CompletableFuture<JoinGroupResponseData> leaderJoinFuture = new
CompletableFuture<>();
- result = sendGenericGroupJoin(joinRequest.setMemberId(leaderId),
leaderJoinFuture);
+ result = sendGenericGroupJoin(
+ joinRequest
+ .setMemberId(leaderJoinResponse.memberId()),
Review Comment:
nit: When there is only one setter, it can stay on the same line. There are
many cases in this file.
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java:
##########
@@ -5837,15 +6007,15 @@ public void testNewMemberTimeoutCompletion() throws
Exception {
.build();
GenericGroup group = context.createGenericGroup("group-id");
- JoinGroupRequestData joinRequest = new JoinGroupRequestBuilder()
+ CompletableFuture<JoinGroupResponseData> joinFuture = new
CompletableFuture<>();
+ CoordinatorResult<Void, Record> result =
context.sendGenericGroupJoin(new JoinGroupRequestBuilder()
Review Comment:
nit: I would be better to format it as follow:
```
CoordinatorResult<Void, Record> result = context.sendGenericGroupJoin(
new JoinGroupRequestBuilder()
.withGroupId("group-id")
.withMemberId(UNKNOWN_MEMBER_ID)
.withDefaultProtocolTypeAndProtocols()
.withSessionTimeoutMs(context.genericGroupNewMemberJoinTimeoutMs
+ 5000)
.build(),
joinFuture
);
```
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java:
##########
@@ -843,15 +886,19 @@ public JoinGroupResponseData
setupGroupWithPendingMember(GenericGroup group) thr
// Start the join for the second member
CompletableFuture<JoinGroupResponseData> followerJoinFuture = new
CompletableFuture<>();
CoordinatorResult<Void, Record> result = sendGenericGroupJoin(
- joinRequest.setMemberId(UNKNOWN_MEMBER_ID),
+ joinRequest
+ .setMemberId(UNKNOWN_MEMBER_ID),
followerJoinFuture
);
assertTrue(result.records().isEmpty());
assertFalse(followerJoinFuture.isDone());
CompletableFuture<JoinGroupResponseData> leaderJoinFuture = new
CompletableFuture<>();
- result = sendGenericGroupJoin(joinRequest.setMemberId(leaderId),
leaderJoinFuture);
+ result = sendGenericGroupJoin(
+ joinRequest
+ .setMemberId(leaderJoinResponse.memberId()),
+ leaderJoinFuture);
Review Comment:
nit: I have noticed that we are not consistent about where we put the
closing parenthesis.
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java:
##########
@@ -5837,15 +6007,15 @@ public void testNewMemberTimeoutCompletion() throws
Exception {
.build();
GenericGroup group = context.createGenericGroup("group-id");
- JoinGroupRequestData joinRequest = new JoinGroupRequestBuilder()
+ CompletableFuture<JoinGroupResponseData> joinFuture = new
CompletableFuture<>();
+ CoordinatorResult<Void, Record> result =
context.sendGenericGroupJoin(new JoinGroupRequestBuilder()
Review Comment:
This code will go away when you change to using `JoinResult`.
--
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
