ahuang98 commented on code in PR #17930:
URL: https://github.com/apache/kafka/pull/17930#discussion_r1917052248
##########
raft/src/main/java/org/apache/kafka/raft/RaftUtil.java:
##########
@@ -546,8 +546,11 @@ public static AddRaftVoterResponseData addVoterResponse(
Errors error,
String errorMessage
) {
- errorMessage = errorMessage == null ? error.message() : errorMessage;
-
+ if (errorMessage == null) {
+ errorMessage = !Errors.NONE.equals(error) ? error.message() : null;
+ } else if (Errors.NONE.equals(error)) {
+ errorMessage = null;
+ }
Review Comment:
ah, I was thinking just
```
if (errorMessage == null && error != Errors.NONE) { errorMessage =
error.message(); }
```
is sufficient without the second `else if` - if `error == Errors.NONE` then
we can take the errorMessage passed in (which is always null).
I do see how this is more explicit, just not sure if it's necessary. If
someone were to misuse the error message for the Errors.NONE case, we would be
overriding that without surfacing that either.
--
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
