terrymanu commented on PR #38786:
URL: https://github.com/apache/shardingsphere/pull/38786#issuecomment-4610533031
### Decision
- **Merge Verdict: Mergeable**
- **Reviewed Scope:** Latest PR head
`37ad11301311f3d1873ba3c9e8d8d4c32100e815`; base and local merge-base
`d74d0e1feaba05a90f9396ff963bf92917e717c1`. Local triple-dot file list matched
GitHub `/pulls/38786/files`. Reviewed all 18 changed ShardingSphere-MCP docs
files under `docs/document/content/user-manual/shardingsphere-mcp/`, including
the renamed `client-integration/_index.*.md` pages and new
`client-integration/{codex,claude-code}.*.md` pages.
- **Not Reviewed Scope:** GitHub Actions/check-run status, live MCP startup
against a real database, and unrelated docs outside the ShardingSphere-MCP
user-manual section.
- **Need Expert Review:** No special expert review required; this is
docs-only and does not alter runtime code, SQL parser behavior, config
semantics, or dependencies.
### Basis
- The PR directly addresses the stated documentation goal: it turns client
integration into a navigable section, adds Codex and Claude Code setup
examples, clarifies HTTP/STDIO usage, and keeps Proxy-only rule-change
boundaries separate from direct database usage.
- The new Codex HTTP setup matches local `codex mcp add --help` behavior and
OpenAI's Codex MCP documentation: `codex mcp add <name> --url <URL>`.
- The new Claude Code HTTP, user-scope, project `.mcp.json`, and STDIO
examples match local `claude mcp add --help` behavior and the official Claude
Code MCP documentation.
- The ShardingSphere-MCP distribution references are consistent with the
packaged files: `distribution/mcp/src/main/bin/start.sh` defaults to
`conf/mcp-http.yaml`, `conf/mcp-stdio.yaml` exists, and the documented default
endpoint remains `http://127.0.0.1:18088/mcp`.
- The changed text preserves the important capability boundary:
encryption/masking rule planning is documented for ShardingSphere-Proxy logical
databases, while direct database connections remain limited to metadata and
ordinary SQL access.
- No SQL parser, shared execution path, high-frequency DML/DQL path,
dependency manifest, or runtime configuration behavior is changed.
### Verification
- `git fetch apache master:refs/remotes/apache/master
pull/38786/head:refs/remotes/apache/pr/38786` exited `0`.
- Local PR scope check using `git diff --name-status
<merge-base>..refs/remotes/apache/pr/38786` matched GitHub `/pulls/38786/files`.
- `codex mcp add --help` and `claude mcp add --help` exited `0` and
confirmed the documented command forms.
- `hugo --destination /tmp/shardingsphere-doc-pr38786 --cleanDestinationDir
--cacheDir /tmp/shardingsphere-hugo-cache-pr38786` from `docs/document` exited
`0`; only existing Hugo theme deprecation warnings were emitted.
- Rendered output includes EN/CN pages for `client-integration/`,
`client-integration/codex/`, and `client-integration/claude-code/`, and the
side navigation links to the new pages.
- `markdown-link-check` on changed files exited `1` because it reports
existing Hugo-style relative directory links such as `../developer-appendix/`
as HTTP 400 in source Markdown. I did not treat that as a blocker because Hugo
generated the target pages and navigation successfully.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
