Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-18241: add docs check to CI #18183

Merged
merged 7 commits into from
Dec 18, 2024
Merged

KAFKA-18241: add docs check to CI #18183

merged 7 commits into from
Dec 18, 2024

Conversation

m1a2st
Copy link
Contributor

@m1a2st m1a2st commented Dec 14, 2024

Jira: https://issues.apache.org/jira/browse/KAFKA-18241

We missed to check generated document in CI, thus we should test this

Committer Checklist (excluded from commit message)

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

@github-actions github-actions bot added triage PRs from the community build Gradle build or GitHub Actions small Small PRs labels Dec 14, 2024
# Check if there are any empty files under ./site-docs/generated, If any empty files are found, print an error
# message and list the empty files
run: |
./gradlew clean siteDocTar
Copy link
Member

Choose a reason for hiding this comment

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

It should not call clean, as doing so prevents subsequent steps from accessing the output of the previous build. Perhaps we should merge the siteDocTar task into the preceding step, and this step verify the contents of the generated documentation files.

@@ -118,6 +118,13 @@ jobs:
# --no-scan: For public fork PRs, we won't attempt to publish the scan
run: |
./gradlew --build-cache --info $SCAN_ARG check -x test
./gradlew siteDocTar
Copy link
Member

Choose a reason for hiding this comment

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

I think @chia7712's comment was to move the siteDocTar into this "Compile and validate" and add a subsequent step to verify the contents (e.g., "Check Site Docs"). Having two gradle commands in one step makes it harder to see what failed without digging into the logs.

@github-actions github-actions bot removed the triage PRs from the community label Dec 16, 2024
@@ -137,6 +137,17 @@ jobs:
run: python .github/scripts/rat.py
env:
GITHUB_WORKSPACE: ${{ github.workspace }}
- name: Check generated documentation
if: always()
Copy link
Member

Choose a reason for hiding this comment

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

Hm, always() means this will run even if a previous step failed. Maybe we should remove this so the step is not run in the event that the "Compile and validate" step fails.

Comment on lines 147 to 149
echo "One or more documentation files are empty!" >&2
find ./site-docs/generated -type f -exec grep -L "." {} \; >&2
exit 1
Copy link
Member

Choose a reason for hiding this comment

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

Can you indent the if body by 2 spaces?

@m1a2st
Copy link
Contributor Author

m1a2st commented Dec 18, 2024

Thanks for @mumrah, @chia7712 review, addressed all comments.

Copy link
Member

@mumrah mumrah left a comment

Choose a reason for hiding this comment

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

LGTM

@mumrah mumrah merged commit 08efe73 into apache:trunk Dec 18, 2024
5 of 6 checks passed
ijuma added a commit to ijuma/kafka that referenced this pull request Dec 19, 2024
…e-old-protocol-versions

* apache-github/trunk: (25 commits)
  KAFKA-18270: FindCoordinator v0 incorrectly tagged as deprecated (apache#18262)
  KAFKA-18284: Add group coordinator records for Streams rebalance protocol (apache#18228)
  MINOR: Fix flaky state updater test (apache#18253)
  MINOR: improve StreamsResetter logging (apache#18237)
  KAFKA-18227: Ensure v2 partitions are not added to last transaction during upgrade (apache#18176)
  Add IT for share consumer with duration base offet auto reset (apache#18251)
  KAFKA-18283: Add StreamsGroupDescribe RPC definitions (apache#18230)
  KAFKA-18241: add docs check to CI (apache#18183)
  KAFKA-18223 Improve flaky test report (apache#18212)
  MINOR Remove triage label in nightly job (apache#18147)
  KAFKA-18294 Remove deprecated SourceTask#commitRecord (apache#18260)
  KAFKA-18264 Remove NotLeaderForPartitionException (apache#18211)
  KAFKA-13722: Refactor SerdeGetter (apache#18242)
  KAFKA-18094 Remove deprecated TopicListing(String, Boolean) (apache#18248)
  KAFKA-18282: Add StreamsGroupHeartbeat RPC definitions (apache#18227)
  KAFKA-18026: KIP-1112 migrate KTableSuppressProcessorSupplier (apache#18150)
  KAFKA-18026: transition KTable#filter impl to use processor wrapper (apache#18205)
  KAFKA-18293 Remove `org.apache.kafka.common.security.oauthbearer.secured.OAuthBearerLoginCallbackHandler` and `org.apache.kafka.common.security.oauthbearer.secured.OAuthBearerValidatorCallbackHandler` (apache#18244)
  MINOR: add assertion about groupEpoch and targetAssignmentEpoch to testConsumerGroups (apache#18203)
  KAFKA-17960; PlaintextAdminIntegrationTest.testConsumerGroups fails with CONSUMER group protocol (apache#18234)
  ...
tedyu pushed a commit to tedyu/kafka that referenced this pull request Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Gradle build or GitHub Actions ci-approved small Small PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants