terrymanu commented on PR #39180: URL: https://github.com/apache/shardingsphere/pull/39180#issuecomment-5011413057
### Summary **Review Result: Mergeable** The PR is mergeable from a code-correctness perspective. It continues the MCP package-boundary refactoring from [PR #39178](https://github.com/apache/shardingsphere/pull/39178), groups tool/resource/prompt/completion contracts by capability, and moves the completion SPI into `mcp.spi`. No functional regression, dependency-direction reversal, stale ServiceLoader registration, or unrelated behavioral change was found. CI not reviewed by request; this result covers code correctness only and does not represent the final CI merge-gate status. ### Evidence - The latest commit is [`e203bd4`](https://github.com/apache/shardingsphere/commit/e203bd4c8df63e9428ebafcead7743a88d5632bb), with base/merge-base `6adf33227f732ff170deae51479c508286c660f8`. - The GitHub file list matches the local three-dot diff scope, covering 198 files. - Production Java semantic changes are limited to package/import updates; migrated types preserve all non-package/import content. - `MCPCompletionProvider` is now located at `mcp/api/src/main/java/org/apache/shardingsphere/mcp/spi/MCPCompletionProvider.java:18`, and consumers use it through `MCPCompletionProviderLoader`. - All six ServiceLoader registrations now use `org.apache.shardingsphere.mcp.spi.MCPCompletionProvider`; no old FQNs or resource paths remain. - The API module has no reverse dependency on support/core/features and introduces no MCP SDK dependency. - The English and Chinese MCP development documentation describes the updated capability and SPI boundaries at `docs/document/content/dev-manual/mcp.en.md:15` and `docs/document/content/dev-manual/mcp.en.md:26`. - Verification on the latest head passed: - MCP API/support/core/features/bootstrap tests: exit 0 - `mcp-e2e` `test-compile`: exit 0 - Spotless check: exit 0 - Checkstyle: exit 0 ### Review Details - Review Focus: Code Correctness Review - Reviewed Scope: MCP API, SPI, support, core, features, bootstrap, E2E, tests, and development documentation; head `e203bd4c8df63e9428ebafcead7743a88d5632bb`; the GitHub file list matches the local scope. - Not Reviewed Scope: GitHub Actions, check-runs, and CI logs; SQL dialects, protocol semantics, runtime state, and dependency versions were not changed. - Release Note / User Docs: The MCP development documentation was updated. This is a package-boundary refactor in the 5.5.4-SNAPSHOT development line, and no compatibility blocker for a released MCP artifact was identified. -- 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]
