terrymanu commented on PR #38819: URL: https://github.com/apache/shardingsphere/pull/38819#issuecomment-4637385071
### Summary - **Merge Decision: Mergeable** - **Reason:** The PR is a scoped comment-only refactor that removes redundant Javadocs from overridden methods while preserving the public contracts on the parent interfaces and passing the scoped style gates. ### Evidence - The stated goal matches the repository rule in `CODE_OF_CONDUCT.md:81`: overriding methods do not need Javadoc, while public/user-facing contracts remain documented at the declaring API/SPI level. - The diff is deletion-only: 183 removed lines, 0 added lines, across redundant Javadoc blocks immediately before `@Override`; no method signatures, constructors, fields, branches, cache keys, parser grammar, visitor logic, or runtime behavior changed. - The removed implementation comments are covered by parent declarations, for example `mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/database/spi/MCPMetadataQueryFacade.java:37`, `mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/database/spi/MCPFeatureCapabilityFacade.java:31`, `mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/database/spi/MCPFeatureExecutionFacade.java:28`, `mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/spi/MCPWorkflowValidationHandler.java:34`, and `parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/generic/AliasAvailable.java:44`. - Shared-path and parser risk were checked: `parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/item/ExpressionProjectionSegment.java:78` remains behavior-identical, so no SQL syntax, dialect-family, name-resolution, fallback, disabled-flag, or AST compatibility review is triggered beyond confirming the comment-only diff. - No substantive unrelated changes were found; the local triple-dot file list matched GitHub `/pulls/38819/files`. ### Review Details - **Reviewed Scope:** Latest PR head `c116b788128ce27e620b2caf3f7c3817ece0c44f`, base/merge-base `0c04936a28773cf97d589606c3e437af52a95c47`; reviewed all 8 GitHub-listed files in `kernel/transaction/api`, `kernel/transaction/type/xa/spi`, `mcp/core`, `mcp/features/encrypt`, `mcp/features/mask`, `mcp/support`, and `parser/sql/statement/core`; local file list matched GitHub `/pulls/38819/files`. - **Not Reviewed Scope:** GitHub Actions/check-run status was intentionally not reviewed; full `clean install` and unit tests were not run because the PR changes comments only and contains no executable code or test behavior change. - **Verification:** `./mvnw -pl kernel/transaction/api,kernel/transaction/type/xa/spi,mcp/core,mcp/features/encrypt,mcp/features/mask,mcp/support,parser/sql/statement/core -am spotless:check checkstyle:check -Pcheck -T1C` exited 0; `git diff --check $(git merge-base apache/master apache/pr/38819)..apache/pr/38819` exited 0. -- 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]
