terrymanu commented on PR #38787: URL: https://github.com/apache/shardingsphere/pull/38787#issuecomment-4610797058
### Decision - **Merge Verdict: Mergeable** - **Reviewed Scope:** Latest PR head `8f8b3502e4a489b2f71d689795ab6e6f4e1075d3`; base/merge-base `f315a1dc7ed09d3e21e6c98c4aeeb9fa378c3462`. Reviewed all 22 GitHub-listed files under `docs/document/content/user-manual/shardingsphere-mcp/`; local triple-dot file list matched GitHub `/pulls/38787/files`. - **Not Reviewed Scope:** GitHub Actions/check-runs, unrelated docs outside the changed MCP user-manual pages, and exhaustive runtime behavior beyond contract checks against MCP implementation/descriptors. - **Need Expert Review:** No dedicated security, performance, concurrency, parser, or protocol expert review required for this docs-only change; standard MCP/docs maintainer review is sufficient. ### Basis - The PR directly matches its stated goal: it shifts the user-facing MCP docs toward natural-language database tasks, moves generic protocol/client snippets into `developer-appendix`, and keeps quick-start/client-integration focused on choosing a client path. - The changed configuration and protocol references remain consistent with implementation contracts: `runtimeDatabases`, `mcp-http.yaml`, `mcp-stdio.yaml`, `STREAMABLE_HTTP`, `STDIO`, `/mcp`, documented resource URI patterns, and core tool names were cross-checked against `mcp/` descriptors, implementation, reference docs, and E2E contract fixtures. - The Proxy-vs-physical-database boundaries remain clear: rule state, encryption, masking, and workflow capabilities are still documented as Proxy-only, while ordinary metadata/query paths remain valid for regular JDBC targets. - Side-effect guidance is preserved: changed pages continue to require preview/review before SQL updates or workflow execution, and sensitive input handling still avoids ordinary docs/chat/log exposure. - No parser, routing, shared execution path, dependency, public API/SPI, or high-frequency `ConcurrentHashMap#computeIfAbsent` path is touched, so those blast-radius gates are not triggered. - No substantive unrelated changes were found; the scope is a coherent MCP user documentation refinement. ### Verification - `git fetch apache master:refs/remotes/apache/master pull/38787/head:refs/remotes/apache/pr/38787` exited `0`. - `git diff --name-status $(git merge-base apache/master apache/pr/38787)..apache/pr/38787` matched GitHub’s 22-file PR scope. - `git diff --check $(git merge-base apache/master apache/pr/38787)..apache/pr/38787 -- docs/document/content/user-manual/shardingsphere-mcp` exited `0`. - `hugo --cleanDestinationDir --destination /tmp/shardingsphere-pr38787-document` from `docs/document` exited `0`; only existing theme deprecation warnings were emitted. - Generated output spot-check confirmed the renamed `developer-appendix` pages and quick-start links to Codex/Claude Code render in both English and Chinese. - `git status --short` exited `0` with no repository changes left by verification. -- 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]
