terrymanu commented on PR #39172:
URL: https://github.com/apache/shardingsphere/pull/39172#issuecomment-5003050629

   ### Summary
   
   **Review Result: Mergeable**
   
   The latest PR correctly performs a package-boundary refactor. The result is 
code-scope only; CI was not reviewed by request.
   
   ### Evidence
   
   - The eight handler-facing exceptions are now under `mcp.api.exception`, for 
example 
`mcp/api/src/main/java/org/apache/shardingsphere/mcp/api/exception/MCPInvalidRequestException.java:18`.
   - `MCPSuccessPayload` is now under `mcp.api.payload` without changing its 
contract or behavior: 
`mcp/api/src/main/java/org/apache/shardingsphere/mcp/api/payload/MCPSuccessPayload.java:18`.
   - All affected consumers use the new package imports, including transport 
error conversion: 
`mcp/bootstrap/src/main/java/org/apache/shardingsphere/mcp/bootstrap/transport/MCPTransportErrorFactory.java:25`.
   - `MCPErrorPayload` remains in the protocol/core layer, preserving the 
existing error transport boundary: 
`mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/protocol/error/MCPErrorPayload.java:18`.
   - The 204-file diff contains only package declarations and import updates. 
No control flow, payload structure, exception mapping, protocol behavior, or 
configuration changed.
   - No legacy `mcp.api.protocol.exception` or `mcp.api.protocol.payload` 
references remain.
   
   ### Review Details
   
   - Review Focus: Code Correctness Review. CI not reviewed by request.
   - Reviewed Scope: 204 files; latest head 
`91a99271877187c1a956f839f0540434f3c3d499`; base and merge-base 
`9bd71ec6a51bfb857047d2a952389e80d6458dcb`. The local file list matched 
GitHub’s `/pulls/39172/files`.
   - Not Reviewed Scope: GitHub Actions, check runs, and external MCP runtime 
deployment.
   - Verification: Spotless passed; Checkstyle passed; MCP and MCP E2E test 
sources compiled; MCP unit tests passed with 290 tests, 0 failures, and 0 
errors.
   - Release Note / User Docs: Not required. This is an internal package 
relocation with no user-visible behavior, configuration, protocol, or migration 
impact.


-- 
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]

Reply via email to