terrymanu commented on PR #38976: URL: https://github.com/apache/shardingsphere/pull/38976#issuecomment-4856512446
### Summary **Review Result: Mergeable** Reason: Code-scope review of the last two commits only found no required changes. This result is code-scope only; CI was not reviewed by request. ### Evidence - The reviewed commits are `43fd6caf66b026ab537a7bec3bb7eb693dc4d27f` and `42654e8aa9e6373813a4916faa6ed729c852c815`. - The workflow support refactors preserve the previous control flow: validation still checks preconditions before creating reports, DistSQL rule-query fallback still only suppresses unsupported-DistSQL failures, and execution-mode/session lifecycle handling remains unchanged. - Error and recovery payload mappings keep the previous matching order, so subtype-specific mappings still win before generic SQL/runtime mappings. - Descriptor validation helper extraction preserves the previous schema-property lookup and required-input checks, with direct tests added for the new public utility methods. - Test helper extraction in MCP bootstrap, workflow execution, JDBC metadata loader, and MCP E2E support removes repeated setup without dropping the existing scenario assertions. ### Review Details - Review Focus: Code Correctness Review. CI not reviewed by request. - Reviewed Scope: only the last two commits, from parent `262f148ff537be30dc12ba44bf778f8ea3eb1ecc` through PR head `42654e8aa9e6373813a4916faa6ed729c852c815`; local merge-base with `master` is `1377e41fd01bf4213dc6e384f19914139a959426`. - Reviewed Files/Modules: 35 files across `mcp/core`, `mcp/support`, `mcp/features/*`, `mcp/bootstrap` tests, and `test/e2e/mcp`. The GitHub PR file list is broader than this review scope; the first `/pulls/files` page alone contains 100 files, so it intentionally does not match the two-commit scope requested here. - Not Reviewed Scope: earlier PR commits, documentation changes outside the last two commits, CI/check results, and runtime E2E execution. - Verification: Inspected public GitHub PR metadata, commit metadata, review/comment endpoints, and the immutable local two-commit diff. No GitHub-visible reviews or comments were present. Local Maven/checkstyle/test commands were not run; this was a static latest-two-commits code review. - Release Note / User Docs: Not required for the reviewed scope because these two commits are internal refactor/test-helper changes and do not change user-facing behavior or documentation. -- 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]
