junrao commented on code in PR #21019:
URL: https://github.com/apache/kafka/pull/21019#discussion_r2755909451
##########
clients/src/main/java/org/apache/kafka/common/protocol/types/ArrayOf.java:
##########
@@ -25,39 +25,21 @@
* Represents a type for an array of a particular type
*/
public class ArrayOf extends DocumentedType {
-
Review Comment:
Since we kept the BYTES implementation, we already have different patterns
for implementing nullable types. So, instead of unifying the pattern, the goal
should probably be to avoid code duplication. Since there is no code
duplication in the existing ArrayOf, we can just keep it as it is. Ditto for
CompactArraryOf.
##########
clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java:
##########
@@ -178,7 +176,7 @@ public BoundField[] fields() {
return this.fields;
}
- protected boolean tolerateMissingFieldsWithDefaults() {
+ public boolean tolerateMissingFieldsWithDefaults() {
Review Comment:
Why is this change needed?
##########
clients/src/main/java/org/apache/kafka/common/protocol/types/Type.java:
##########
@@ -455,6 +458,14 @@ public String documentation() {
}
};
+ public String stringRead(ByteBuffer buffer, short length) {
Review Comment:
Could this be static? Also, it seems that we can just reuse
compactStringRead(). It's useful to do the length > Short.MAX_VALUE validation
for both compact and non-compact string.
--
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]
