mimaison commented on code in PR #17681:
URL: https://github.com/apache/kafka/pull/17681#discussion_r1858117155
##########
clients/src/main/resources/common/message/DescribeClientQuotasResponse.json:
##########
@@ -36,7 +36,7 @@
"about": "The entity name, or null if the default." }
]},
{ "name": "Values", "type": "[]ValueData", "versions": "0+",
- "about": "The quota values for the entity.", "fields": [
+ "about": "The quota values for the entity.", "fields": [
Review Comment:
The alignment is still not right here.
##########
clients/src/main/resources/common/message/FetchRequest.json:
##########
@@ -71,15 +71,16 @@
"about": "The replica ID of the follower, or -1 if this request is
from a consumer." },
{ "name": "ReplicaEpoch", "type": "int64", "versions": "15+", "default":
"-1",
"about": "The epoch of this follower, or -1 if not available." }
- ]},
+ ],
+ "about": "The state of the replica in the follower."},
Review Comment:
I think it makes more sense to put the description above the inner fields.
##########
group-coordinator/src/main/resources/common/message/OffsetCommitKey.json:
##########
@@ -19,8 +19,11 @@
"validVersions": "0-1",
"flexibleVersions": "none",
"fields": [
- { "name": "group", "type": "string", "versions": "0-1" },
- { "name": "topic", "type": "string", "versions": "0-1" },
- { "name": "partition", "type": "int32", "versions": "0-1" }
+ { "name": "group", "type": "string", "versions": "0-1",
+ "about": "The consumer group id."},
+ { "name": "topic", "type": "string", "versions": "0-1",
+ "about": "The topic name."},
+ { "name": "partition", "type": "int32", "versions": "0-1",
+ "about": "The topic partition id."}
Review Comment:
In other places we use partition "number" or "index" instead of partition id
##########
raft/src/main/resources/common/message/QuorumStateData.json:
##########
@@ -19,14 +19,22 @@
"validVersions": "0-1",
"flexibleVersions": "0+",
"fields": [
- { "name": "ClusterId", "type": "string", "versions": "0" },
- { "name": "LeaderId", "type": "int32", "versions": "0+", "default": "-1" },
- { "name": "LeaderEpoch", "type": "int32", "versions": "0+", "default":
"-1" },
- { "name": "VotedId", "type": "int32", "versions": "0+", "default": "-1" },
- { "name": "VotedDirectoryId", "type": "uuid", "versions": "1+" },
- { "name": "AppliedOffset", "type": "int64", "versions": "0" },
- { "name": "CurrentVoters", "type": "[]Voter", "versions": "0",
"nullableVersions": "0", "fields": [
- { "name": "VoterId", "type": "int32", "versions": "0" }
+ { "name": "ClusterId", "type": "string", "versions": "0",
+ "about": "The cluster id."},
+ { "name": "LeaderId", "type": "int32", "versions": "0+", "default": "-1",
+ "about": "The leader id."},
+ { "name": "LeaderEpoch", "type": "int32", "versions": "0+", "default":
"-1",
+ "about": "The leader epoch."},
+ { "name": "VotedId", "type": "int32", "versions": "0+", "default": "-1",
+ "about": "The voted id."},
+ { "name": "VotedDirectoryId", "type": "uuid", "versions": "1+",
+ "about": "The voted directory id."},
+ { "name": "AppliedOffset", "type": "int64", "versions": "0",
+ "about": "The applied offset."},
+ { "name": "CurrentVoters", "type": "[]Voter", "versions": "0",
"nullableVersions": "0",
+ "about": "The current voters.", "fields": [
+ { "name": "VoterId", "type": "int32", "versions": "0",
+ "about": "The voter id."}
Review Comment:
The alignment seems off
##########
clients/src/main/resources/common/message/FetchSnapshotResponse.json:
##########
@@ -36,35 +36,38 @@
{ "name": "ErrorCode", "type": "int16", "versions": "0+",
"about": "The error code, or 0 if there was no fetch error." },
{ "name": "SnapshotId", "type": "SnapshotId", "versions": "0+",
- "about": "The snapshot endOffset and epoch fetched", "fields": [
- { "name": "EndOffset", "type": "int64", "versions": "0+" },
- { "name": "Epoch", "type": "int32", "versions": "0+" }
+ "about": "The snapshot endOffset and epoch fetched.", "fields": [
+ { "name": "EndOffset", "type": "int64", "versions": "0+",
+ "about": "The snapshot end offset."},
+ { "name": "Epoch", "type": "int32", "versions": "0+",
+ "about": "The snapshot epoch."}
]
},
{ "name": "CurrentLeader", "type": "LeaderIdAndEpoch",
"versions": "0+", "taggedVersions": "0+", "tag": 0, "fields": [
{ "name": "LeaderId", "type": "int32", "versions": "0+",
"entityType": "brokerId",
"about": "The ID of the current leader or -1 if the leader
is unknown."},
{ "name": "LeaderEpoch", "type": "int32", "versions": "0+",
- "about": "The latest known leader epoch"}
- ]
+ "about": "The latest known leader epoch."}
+ ],
+ "about": "The leader of the partition at the time of the
snapshot."
Review Comment:
Again I'd put the description above the inner fields
##########
transaction-coordinator/src/main/resources/common/message/TransactionLogValue.json:
##########
@@ -23,26 +23,28 @@
"flexibleVersions": "1+",
"fields": [
{ "name": "ProducerId", "type": "int64", "versions": "0+",
- "about": "Producer id in use by the transactional id"},
+ "about": "Producer id in use by the transactional id."},
{ "name": "PreviousProducerId", "type": "int64", "taggedVersions": "1+",
"tag": 0, "default": -1,
- "about": "Producer id used by the last committed transaction"},
+ "about": "Producer id used by the last committed transaction."},
{ "name": "NextProducerId", "type": "int64", "taggedVersions": "1+",
"tag": 1, "default": -1,
- "about": "Latest producer ID sent to the producer for the given
transactional ID"},
+ "about": "Latest producer ID sent to the producer for the given
transactional ID."},
{ "name": "ProducerEpoch", "type": "int16", "versions": "0+",
- "about": "Epoch associated with the producer id"},
+ "about": "Epoch associated with the producer id."},
{ "name": "TransactionTimeoutMs", "type": "int32", "versions": "0+",
- "about": "Transaction timeout in milliseconds"},
+ "about": "Transaction timeout in milliseconds."},
{ "name": "TransactionStatus", "type": "int8", "versions": "0+",
- "about": "TransactionState the transaction is in"},
- { "name": "TransactionPartitions", "type": "[]PartitionsSchema",
"versions": "0+", "nullableVersions": "0+",
- "about": "Set of partitions involved in the transaction", "fields": [
- { "name": "Topic", "type": "string", "versions": "0+"},
- { "name": "PartitionIds", "type": "[]int32", "versions": "0+"}]},
+ "about": "TransactionState the transaction is in."},
+ { "name": "TransactionPartitions", "type": "[]PartitionsSchema",
"versions": "0+", "nullableVersions": "0+", "fields": [
+ { "name": "Topic", "type": "string", "versions": "0+",
+ "about": "Topic involved in the transaction."},
+ { "name": "PartitionIds", "type": "[]int32", "versions": "0+",
+ "about": "Partition ids involved in the transaction."}],
+ "about": "Partitions involved in the transaction."},
Review Comment:
Let's put the description before the inner fields.
##########
group-coordinator/src/main/resources/common/message/GroupMetadataValue.json:
##########
@@ -22,26 +22,40 @@
"validVersions": "0-4",
"flexibleVersions": "4+",
"fields": [
- { "name": "protocolType", "versions": "0+", "type": "string"},
- { "name": "generation", "versions": "0+", "type": "int32" },
- { "name": "protocol", "versions": "0+", "type": "string",
"nullableVersions": "0+" },
- { "name": "leader", "versions": "0+", "type": "string",
"nullableVersions": "0+" },
- { "name": "currentStateTimestamp", "versions": "2+", "type": "int64",
"default": -1, "ignorable": true},
- { "name": "members", "versions": "0+", "type": "[]MemberMetadata" }
+ { "name": "protocolType", "versions": "0+", "type": "string",
+ "about": "The protocol type."},
Review Comment:
The alignment seems off here as well
--
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
