Skip to content

MINOR: comment apikey types in generated switch#8201

Merged
mimaison merged 1 commit intoapache:trunkfrom
dnwe:generator-comment-apikey-types
Mar 15, 2020
Merged

MINOR: comment apikey types in generated switch#8201
mimaison merged 1 commit intoapache:trunkfrom
dnwe:generator-comment-apikey-types

Conversation

@dnwe
Copy link
Copy Markdown
Member

@dnwe dnwe commented Mar 2, 2020

As a developer, it would be convenient if the generated
{request,response}HeaderVersion case statements in ApiMessageType.java
included a comment to remind me which type each of them is so I don't
need to manually cross-reference the newer/rarer ones.

Also include commented lines for the two special cases around
ApiVersionsResponse and ControllerShutdownRequest which are hardcoded in
the ApiMessageTypeGenerator.java and not covered by the message format
json files.

Before:

    public short requestHeaderVersion(short _version) {
        switch (apiKey) {
            case 0:
                return (short) 1;
            case 1:
                return (short) 1;
            case 2:
                return (short) 1;
            case 3:
                if (_version >= 9) {
                    return (short) 2;
                } else {
                    return (short) 1;
                }
            // ...etc

After:

    public short requestHeaderVersion(short _version) {
        switch (apiKey) {
            case 0: // Produce
                return (short) 1;
            case 1: // Fetch
                return (short) 1;
            case 2: // ListOffset
                return (short) 1;
            case 3: // Metadata
                if (_version >= 9) {
                    return (short) 2;
                } else {
                    return (short) 1;
                }
            // ...etc

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

As a developer, it would be convenient if the generated
{request,response}HeaderVersion case statements in ApiMessageType.java
included a comment to remind me which type each of them is so I don't
need to manually cross-reference the newer/rarer ones.

Also include commented lines for the two special cases around
ApiVersionsResponse and ControllerShutdownRequest which are hardcoded in
the ApiMessageTypeGenerator.java and not covered by the message format
json files.

Before:
```java
    public short requestHeaderVersion(short _version) {
        switch (apiKey) {
            case 0:
                return (short) 1;
            case 1:
                return (short) 1;
            case 2:
                return (short) 1;
            case 3:
                if (_version >= 9) {
                    return (short) 2;
                } else {
                    return (short) 1;
                }
            // ...etc
```

After:
```java
    public short requestHeaderVersion(short _version) {
        switch (apiKey) {
            case 0: // Produce
                return (short) 1;
            case 1: // Fetch
                return (short) 1;
            case 2: // ListOffset
                return (short) 1;
            case 3: // Metadata
                if (_version >= 9) {
                    return (short) 2;
                } else {
                    return (short) 1;
                }
            // ...etc
```

Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
@mimaison
Copy link
Copy Markdown
Member

retest this please

Copy link
Copy Markdown
Member

@mimaison mimaison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mimaison mimaison merged commit ddd3dfb into apache:trunk Mar 15, 2020
ijuma added a commit to confluentinc/kafka that referenced this pull request Mar 17, 2020
* apache-github/trunk: (39 commits)
  MINOR: cleanup and add tests to StateDirectoryTest (apache#8304)
  HOTFIX: StateDirectoryTest should use Set instead of List (apache#8305)
  MINOR: Fix build and JavaDoc warnings (apache#8291)
  MINOR: Fix kafka.server.RequestQuotaTest missing new ApiKeys. (apache#8302)
  KAFKA-9712: Catch and handle exception thrown by reflections scanner (apache#8289)
  KAFKA-9670; Reduce allocations in Metadata Response preparation (apache#8236)
  MINOR: fix Scala 2.13 build error introduced in apache#8083 (apache#8301)
  MINOR: enforce non-negative invariant for checkpointed offsets (apache#8297)
  MINOR: comment apikey types in generated switch (apache#8201)
  MINOR: Fix typo in CreateTopicsResponse.json (apache#8300)
  KIP-546: Implement describeClientQuotas and alterClientQuotas. (apache#8083)
  KAFKA-6647: Do note delete the lock file while holding the lock (apache#8267)
  KAFKA-9677: Fix consumer fetch with small consume bandwidth quotas (apache#8290)
  KAFKA-9533: Fix JavaDocs of KStream.transformValues (apache#8298)
  MINOR: reuse pseudo-topic in FKJoin (apache#8296)
  KAFKA-6145: Pt 2. Include offset sums in subscription (apache#8246)
  KAFKA-9714; Eliminate unused reference to IBP in `TransactionStateManager` (apache#8293)
  KAFKA-9718; Don't log passwords for AlterConfigs in request logs (apache#8294)
  KAFKA-8768: DeleteRecords request/response automated protocol (apache#7957)
  KAFKA-9685: Solve Set concatenation perf issue in AclAuthorizer
  ...
@dnwe dnwe deleted the generator-comment-apikey-types branch March 24, 2024 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants