dajac commented on code in PR #15818:
URL: https://github.com/apache/kafka/pull/15818#discussion_r1582660947
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorResult.java:
##########
@@ -80,10 +86,28 @@ public CoordinatorResult(
List<U> records,
T response,
CompletableFuture<Void> appendFuture
+ ) {
+ this(records, response, appendFuture, appendFuture == null);
+ }
+
+ /**
+ * Constructs a Result with records, a response, an append-future, and
replayRecords.
+ *
+ * @param records
+ * @param response
+ * @param appendFuture
+ * @param replayRecords
+ */
+ public CoordinatorResult(
+ List<U> records,
+ T response,
+ CompletableFuture<Void> appendFuture,
+ boolean replayRecords
) {
this.records = Objects.requireNonNull(records);
this.response = response;
this.appendFuture = appendFuture;
+ this.replayRecords = Objects.requireNonNull(replayRecords);
Review Comment:
`Objects.requireNonNull` does not seem necessary here as `replayRecords` is
a primitive type.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorResult.java:
##########
@@ -66,7 +72,7 @@ public CoordinatorResult(
List<U> records,
CompletableFuture<Void> appendFuture
) {
- this(records, null, appendFuture);
+ this(records, null, appendFuture, appendFuture == null);
Review Comment:
I don't like this. I think that we should rather be explicit everywhere
about it. How about saying that `replayRecords` is `true` unless specified
otherwise?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorResult.java:
##########
@@ -80,10 +86,28 @@ public CoordinatorResult(
List<U> records,
T response,
CompletableFuture<Void> appendFuture
+ ) {
+ this(records, response, appendFuture, appendFuture == null);
+ }
+
+ /**
+ * Constructs a Result with records, a response, an append-future, and
replayRecords.
+ *
+ * @param records
+ * @param response
+ * @param appendFuture
+ * @param replayRecords
Review Comment:
nit: Let's add description for all the fields.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorResult.java:
##########
@@ -80,10 +86,28 @@ public CoordinatorResult(
List<U> records,
T response,
CompletableFuture<Void> appendFuture
+ ) {
+ this(records, response, appendFuture, appendFuture == null);
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
