hachikuji commented on pull request #11733: URL: https://github.com/apache/kafka/pull/11733#issuecomment-1050566273
For this [comment](https://github.com/apache/kafka/pull/11733#discussion_r812044247): > If the version is `0` then this is guaranteed to be the default value `0` so the serialization will succeed. This is true because we only write these values in the response when the operation success. If the operation fails then we skip writing these values and instead just write the error code. This is a bit subjective, so feel free to disregard, but I do feel like some of the implicit assumptions might be causing some unnecessary obscurity. This is one case where a version check might actually be clearer and prevent the need for the extra comment. A second case is implicitly setting RECOVERED in `PendingPartitionChange`. I had a comment about this [here](https://github.com/apache/kafka/pull/11733/files#r805005385), which might have been missed. This is fine at the moment because the current patch does not do any actual recovery operation, but I think we should reconsider it when we do. Otherwise I do think it's easy to overlook the implication when making other partition state changes. -- 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
