Skip to content

Docs: own and sync ClickHouse documentation#1929

Open
Blargian wants to merge 1 commit into
ClickHouse:mainfrom
Blargian:agent/own-clickhouse-docs
Open

Docs: own and sync ClickHouse documentation#1929
Blargian wants to merge 1 commit into
ClickHouse:mainfrom
Blargian:agent/own-clickhouse-docs

Conversation

@Blargian

Copy link
Copy Markdown
Member

Summary

Make ClickHouse/clickhouse-go the source of truth for its public ClickHouse documentation.

  • Copy the current Go documentation slice from ClickHouse/ClickHouse@21ed77718a5f454680497924d68e9f60a41e026c into docs/, with trailing whitespace normalized.
  • Add a scoped Mintlify verification workflow for docs changes.
  • Add a one-way sync workflow that opens or refreshes the corresponding ClickHouse/ClickHouse docs PR after a release, manual dispatch, or merge carrying the sync-docs label.

The published URLs and aggregator path remain unchanged. Future content changes can be reviewed alongside the client implementation and then synchronized automatically.

After merge, maintainers should confirm the sync-docs label exists and the Workflow Auth GitHub App credentials are available to this repository.

Validation

  • All workflows pass actionlint v1.7.7 and YAML parsing.
  • git diff --check passes.
  • The migrated slice matches the current aggregator source after trailing-whitespace normalization.

@Blargian Blargian changed the title Own and sync ClickHouse documentation Docs: own and sync ClickHouse documentation Jul 21, 2026
@Blargian
Blargian marked this pull request as ready for review July 22, 2026 09:18
@Blargian

Copy link
Copy Markdown
Member Author

@kavirajk the docs verify workflow added here also uses lychee to check the links within the context of the entire docs site. Will need to exclude the newly added files from the existing link checker you have set up.

# Label on a merged PR in THIS repo that triggers the expedited path.
SYNC_LABEL: sync-docs
# Which releases trigger a sync: major (X.0.0), minor (X.Y.0), or all.
RELEASE_SCOPE: all

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Should fix — RELEASE_SCOPE default contradicts the header comment

The header comment (line 9) states releases sync "major-only by default," but RELEASE_SCOPE is set to all, so every non-prerelease release triggers a sync attempt — a maintainer tuning cadence from the comment will be misled.

The empty-PR guard keeps this harmless in practice, so pick the intended default and make the two agree.

Suggested change
RELEASE_SCOPE: all
RELEASE_SCOPE: major

# scripts (ci/jobs/scripts/docs), so any check scripts come along for free.
- name: Fetch the docs check driver
run: |
curl -fsSL -o /tmp/mintlify_docs_check.py \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Should fix — Pin the fetched check script and container image

This step curls a script from master and pipes it into python3, and the job runs in the untagged clickhouse/docs-builder (i.e. :latest) image — so upstream changes can silently break or alter CI, and the run is not reproducible.

  • Pin the script to a commit SHA (.../ClickHouse/<sha>/ci/jobs/scripts/docs/mintlify_docs_check.py) rather than master.
  • Pin the container to a tag or digest.
  • Add an explicit least-privilege permissions: contents: read block, since on the push: main trigger this executes remote code with whatever the repo's default token scope is.

@github-actions

Copy link
Copy Markdown

🤖 Claude review

Moves the Go client's public docs into docs/ and adds two workflows: docs_verify.yml (scoped Mintlify checks on docs PRs) and docs_sync.yml (one-way sync that opens/refreshes a docs PR on ClickHouse/ClickHouse). No Go code changes.

The sync workflow is well-built for a pull_request_target trigger: it never checks out or executes untrusted PR code, gates the expedited path on merged == true and the sync-docs label (which only write-access users can apply), uses a repo-scoped GitHub App token instead of the ambient GITHUB_TOKEN, and guards against empty PRs with git diff --cached --quiet. I found no correctness or security must-fixes.

Suggestions (non-blocking):

  • RELEASE_SCOPE: all contradicts the header comment's "major-only by default" — align the comment or the value (inline).
  • docs_verify.yml fetches and runs an unpinned script from master inside an untagged container image, with no explicit least-privilege permissions (inline).

Blind spots:

  • The .mdx files and navigation.json are a verbatim upstream mirror; I did not review their prose/example correctness (the PR contract is byte-parity with the aggregator after whitespace normalization).
  • Could not validate the external mintlify_docs_check.py behavior, nor confirm the WORKFLOW_AUTH_PUBLIC_* secrets / sync-docs label exist for this repo — the PR body already flags these as post-merge maintainer actions.

Verdict: ✅ Approve

Inline comments are attached to the relevant lines. This summary updates in place on re-review.

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