mimaison commented on code in PR #17681:
URL: https://github.com/apache/kafka/pull/17681#discussion_r1852192036
##########
clients/src/main/resources/common/message/ReadShareGroupStateResponse.json:
##########
@@ -42,7 +42,8 @@
"about": "The state epoch for this share-partition." },
{ "name": "StartOffset", "type": "int64", "versions": "0+",
"about": "The share-partition start offset, which can be -1 if it is
not yet initialized." },
- { "name": "StateBatches", "type": "[]StateBatch", "versions": "0+",
"fields":[
+ { "name": "StateBatches", "type": "[]StateBatch", "versions": "0+",
+ "about": "The state batches for this partition.", "fields":[
Review Comment:
nit: for consistency should we say `share-partition` instead of just
`partition`?
##########
clients/src/main/resources/common/message/ShareFetchResponse.json:
##########
@@ -54,7 +54,8 @@
"about": "The acknowledge error code, or 0 if there was no
acknowledge error." },
{ "name": "AcknowledgeErrorMessage", "type": "string", "versions":
"0+", "nullableVersions": "0+", "default": "null",
"about": "The acknowledge error message, or null if there was no
acknowledge error." },
- { "name": "CurrentLeader", "type": "LeaderIdAndEpoch", "versions":
"0+", "fields": [
+ { "name": "CurrentLeader", "type": "LeaderIdAndEpoch", "versions":
"0+",
+ "about": "The current leader for this partition.", "fields": [
Review Comment:
In `ShareAcknowledgeResponse` you used `The current leader of the
partition.`. Why are we using a different phrasing here?
##########
group-coordinator/src/main/resources/common/message/ConsumerGroupTargetAssignmentMemberValue.json:
##########
@@ -22,8 +22,10 @@
"fields": [
{ "name": "TopicPartitions", "versions": "0+", "type": "[]TopicPartition",
"about": "The assigned partitions.", "fields": [
- { "name": "TopicId", "versions": "0+", "type": "uuid" },
- { "name": "Partitions", "versions": "0+", "type": "[]int32" }
+ { "name": "TopicId", "versions": "0+", "type": "uuid",
+ "about": "The topic id."},
Review Comment:
The indentation does not look right here. `"about"` should be align with
`"name"`, like it's done above.
This applies to many other files where you added `about` fields.
##########
clients/src/main/resources/common/message/UpdateRaftVoterResponse.json:
##########
@@ -23,15 +23,15 @@
{ "name": "ThrottleTimeMs", "type": "int32", "versions": "0+",
"about": "The duration in milliseconds for which the request was
throttled due to a quota violation, or zero if the request did not violate any
quota." },
{ "name": "ErrorCode", "type": "int16", "versions": "0+",
- "about": "The error code, or 0 if there was no error" },
+ "about": "The error code, or 0 if there was no error." },
{ "name": "CurrentLeader", "type": "CurrentLeader", "versions": "0+",
- "taggedVersions": "0+", "tag": 0, "fields": [
+ "taggedVersions": "0+", "tag": 0, "about": "The current leader of the
group.", "fields": [
Review Comment:
I don't think this is the leader of a "group"
##########
clients/src/main/resources/common/message/WriteShareGroupStateRequest.json:
##########
@@ -38,13 +38,14 @@
"about": "The leader epoch of the share-partition." },
{ "name": "StartOffset", "type": "int64", "versions": "0+",
"about": "The share-partition start offset, or -1 if the start
offset is not being written." },
- { "name": "StateBatches", "type": "[]StateBatch", "versions": "0+",
"fields": [
+ { "name": "StateBatches", "type": "[]StateBatch", "versions": "0+",
+ "about": "The state batches for the partition.", "fields": [
Review Comment:
Should we use `share-partition`?
--
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]
