terrymanu commented on PR #38747:
URL: https://github.com/apache/shardingsphere/pull/38747#issuecomment-4562990065
### Decision
- **Merge Verdict: Mergeable**
- **Reviewed Scope:** PR #38747 latest head
`f5a87630e62402043308ddcad9dfdd81cfce30ab`; base
`88c8f9ae8a70179613b37dee6ee0819738daef56`; local merge-base
`786ac3d6e9587c1576bbcd5f83aa8610ded7c816`. Reviewed all 36 GitHub changed
files, covering MCP official docs under `docs/document/content`, release-doc
wording, `distribution/mcp/pom.xml`, deleted `mcp/README.md` and
`mcp/README_ZH.md`, `mcp/server.json`, and the README-coupled test fixture
updates. Local file list matched GitHub `/pulls/38747/files`.
- **Not Reviewed Scope:** GitHub Actions / CI statuses were intentionally
not reviewed. I did not run a live manual MCP HTTP/STDIO quick-start session or
LLM E2E lanes.
- **Need Expert Review:** No specialized security, parser, concurrency, or
protocol expert review is required for this change set.
### Basis
- The PR directly addresses the stated goal: MCP module README content is
moved into official documentation, the old module README files are removed,
distribution packaging no longer copies those README files, and registry
metadata now points users to the official documentation entry.
- The two non-documentation test/config changes are scoped and justified:
-
`mcp/registry/src/test/java/org/apache/shardingsphere/mcp/registry/MCPRegistryMetadataCommandTest.java`
no longer uses `mcp/README.md` as a project-root sentinel.
-
`agent/core/src/test/java/org/apache/shardingsphere/agent/core/plugin/jar/PluginJarLoaderTest.java`
uses a neutral ignored file name instead of `README.txt`.
- Risk scan did not find unresolved side effects:
- No SQL parser, routing, name-resolution, default-schema, or dialect
behavior is changed.
- No shared Proxy/JDBC DML/DQL hot path is changed, and no
`ConcurrentHashMap#computeIfAbsent` issue is introduced.
- MCP runtime behavior is not changed except distribution packaging no
longer includes the removed README files.
- Distribution still packages `LICENSE`, `NOTICE`, `bin/`, `conf/`,
`lib/`, and runtime directories as expected.
- Adversarial pass:
- Cross-feature risk: Encrypt and Mask documentation is separated and no
feature runtime code path is altered.
- Config-disabled / adjacent path risk: HTTP and STDIO docs/config
references build cleanly; packaging still includes both `mcp-http.yaml` and
`mcp-stdio.yaml`.
- Original symptom risk: scans found no remaining README or old `mcp`
GitHub README URL dependency in Java/XML/JSON/YAML/scripts or the new MCP docs
scope.
### Verification
- `hugo --source docs/document --destination
/tmp/shardingsphere-doc-document-pr38747 --cleanDestinationDir`: passed.
- `hugo --source docs/community --destination
/tmp/shardingsphere-doc-community-pr38747 --cleanDestinationDir`: passed.
- `./mvnw -pl agent/core -am -DskipITs -Dspotless.skip=true
-Dtest=PluginJarLoaderTest test -Dsurefire.failIfNoSpecifiedTests=false -B
-ntp`: passed.
- `./mvnw -pl mcp/registry -am -DskipITs -Dspotless.skip=true
-Dtest=MCPRegistryMetadataCommandTest test
-Dsurefire.failIfNoSpecifiedTests=false -B -ntp`: passed.
- `./mvnw -pl distribution/mcp -am -DskipTests package -B -ntp`: passed.
- `./mvnw spotless:check -Pcheck -T1C -B -ntp`: passed.
- `./mvnw checkstyle:check -Pcheck -T1C -B -ntp`: passed.
--
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]
