KAFKA-20779: KIP-1265 review follow-ups from #21337#22772
Open
ashwinpankaj wants to merge 1 commit into
Open
Conversation
Addresses reviewer feedback (chia7712) on the merged KIP-1265 PR:
- PublicApiViolation.equals: collapse the null-safe branch to
Objects.equals(memberName, that.memberName).
- CascadeValidator.asSuppression: omit "#member" when the finding has
no memberName so class-scope INVALID_SUPERTYPE suppressions no longer
render as "…Owner#null".
- KafkaInternalApiCheckerMojo: add doc comments and @param tags to the
class default constructor and the six setters so
:api-checker:maven-plugin:javadoc is warning-free.
- api-checker/build.gradle: add testLogging { events "passed",
"skipped", "failed" } to the shared subprojects test config for
consistency with the rest of the build.
- Root build.gradle: extend the root `check` task to depend on the
composite-included api-checker's :core:check, :gradle-plugins:check,
and :maven-plugin:check. `.github/workflows/build.yml` runs
./gradlew check on every PR, so this wires the api-checker's
checkstyle + unit tests (including PluginXmlParityTest) into the
standard PR workflow.
- api-checker/README.md: document :maven-plugin:test (including
PluginXmlParityTest), update the layout tree to show the test folder,
and rewrite the "CI integration" section to reflect the new wiring.
./gradlew check -x test — clean, 7m 38s, exercises all 13 api-checker
verification tasks alongside every other module.
./gradlew :api-checker:maven-plugin:javadoc — no warnings.
github-actions
Bot
added
triage
chia7712
reviewed
Jul 9, 2026
chia7712
left a comment
chia7712
left a comment
Member
There was a problem hiding this comment.
@ashwinpankaj thanks for this follow-up. LGTM except for one small comment :)
| String description = "Suppressed " + original.getViolationType() + " in " | ||
| + original.getClassName() + "#" + original.getMemberName() | ||
| + location | ||
| + " — " + original.getDescription() |
Member
There was a problem hiding this comment.
Would you mind using ascii code instead? — -> -
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up nits from @chia7712's review of #21337. None affects
checker behaviour; grouped as a single small PR.
JIRA: KAFKA-20779
Changes
core/…/PublicApiViolation.javaequalsusesObjects.equals(memberName, …)instead of the ternary null-check.core/…/CascadeValidator.javaasSuppressionomits#memberwhen the finding has nomemberName, so class-scopeINVALID_SUPERTYPEsuppressions no longer render as…Owner#null.maven-plugin/…/KafkaInternalApiCheckerMojo.java@paramtags on the class default ctor + six setters;:maven-plugin:javadocis now warning-free.api-checker/build.gradletestLogging { events "passed", "skipped", "failed" }for consistency with the main Kafka build.build.gradlechecktask to depend on:api-checker:core:check,:api-checker:gradle-plugins:check, and:api-checker:maven-plugin:check. Since.github/workflows/build.ymlruns./gradlew checkon every PR, the composite build's checkstyle + unit tests (incl.PluginXmlParityTest) are now exercised in the standard PR workflow.api-checker/README.md:maven-plugin:test; layout tree shows the test folder; "CI integration" section rewritten to reflect the new wiring.Test plan
./gradlew check -x test— clean end-to-end in 7m 38s; all 13 api-checker verification tasks appear in the plan and pass../gradlew :api-checker:maven-plugin:javadoc— zero warnings../gradlew check --dry-runconfirms:api-checker:*:check/:test/:checkstyle*now show up under rootcheck.Reviewers: Chia-Ping Tsai chia7712@gmail.com