terrymanu commented on PR #38891: URL: https://github.com/apache/shardingsphere/pull/38891#issuecomment-4771524553
### Summary **Review Result: Mergeable** **Reason:** This PR is a narrow cleanup that removes the unused 5-argument `loadCreatedTable` overload from `TableMetaDataRefresherLoader`. The remaining create-table path already calls the 6-argument overload with explicit `revisionCandidateSchemas`, no repository references to the removed overload remain, and scoped verification passed. ### Evidence - The current create-table refresh path calls `loadCreatedTable(..., props, createRevisionCandidateSchemas(...))`, so it does not depend on the removed convenience overload (`mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/refresher/pushdown/type/table/CreateTablePushDownMetaDataRefresher.java:48-51`). - `TableMetaDataRefresherLoader` keeps the explicit 6-argument create-table loader and the alter-table loader unchanged, preserving the actual production paths (`mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/refresher/pushdown/type/table/TableMetaDataRefresherLoader.java:55-74`). - `git grep` on the reviewed head found only the two production callers and the remaining method declaration; no in-repository caller uses the removed overload. - No docs, examples, release notes, or distribution files reference `TableMetaDataRefresherLoader`, so this internal helper cleanup does not require user-facing migration notes. ### Review Details - **Reviewed Scope:** PR #38891 latest head `32f782390b0e6b5fab8435a8ad7b17656e945aa0`; base/merge-base `85a1b158974757d743b65cf6e79c78d186b9b36e`. GitHub `/pulls/38891/files` matched the local triple-dot file list. Reviewed the single changed file: `mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/refresher/pushdown/type/table/TableMetaDataRefresherLoader.java`. - **Not Reviewed Scope:** Full repository build, full CI, and E2E image jobs. GitHub `Check - Spotless`, `Check - CheckStyle`, and `Check - License` were successful; broader `CI`/E2E jobs were still in progress at review time. - **Verification:** Initial reviewer command without `-Dsurefire.failIfNoSpecifiedTests=false` exited 1 because `-am` included upstream modules with no matching specified tests. Corrected command passed with exit code 0: `./mvnw -pl mode/core -am -DskipITs -Dcheckstyle.skip -Dspotless.check.skip=true -Dsurefire.failIfNoSpecifiedTests=false -Dtest=CreateTablePushDownMetaDataRefresherTest,AlterTablePushDownMetaDataRefresherTest test`. `git diff --check` passed with exit code 0. - **Release Note / User Docs:** Not required. The PR removes an unused internal overload and does not change documented configuration, SQL behavior, runtime behavior, or user-facing diagnostics. -- 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]
