terrymanu commented on PR #38890: URL: https://github.com/apache/shardingsphere/pull/38890#issuecomment-4771459502
### Summary **Review Result: Mergeable** **Reason:** This PR is a scoped rename for revision-candidate naming. The GitHub file list matches the local triple-dot diff, old repetitive names are fully removed from the reviewed head, all in-repo callers/tests are updated, and scoped Maven verification passed. ### Evidence - The factory path now delegates to `createWithRevisionCandidateSchemas`, and the candidate schema collection is passed unchanged into `GenericSchemaBuilderMaterial` (`infra/common/src/main/java/org/apache/shardingsphere/infra/metadata/database/ShardingSphereDatabaseFactory.java:84-143`, `infra/common/src/main/java/org/apache/shardingsphere/infra/metadata/database/schema/builder/GenericSchemaBuilderMaterial.java:49-53`). - The metadata revision pipeline consumes the renamed material getter without changing control flow (`infra/common/src/main/java/org/apache/shardingsphere/infra/metadata/database/schema/reviser/MetaDataReviseEngine.java:61-69`, `infra/common/src/main/java/org/apache/shardingsphere/infra/metadata/database/schema/reviser/table/TableMetaDataReviseEngine.java:73-80`, `infra/common/src/main/java/org/apache/shardingsphere/infra/metadata/database/schema/reviser/index/IndexReviseEngine.java:51-59`). - The `IndexReviser` parameter rename is synchronized with the encrypt/sharding implementations and test fixture. - `git grep` over the reviewed head found no remaining old names such as `createWithSchemaMetaDataRevisionCandidates`, `schemaMetaDataRevisionCandidateSchemas`, `createSchemaMetaDataRevisionCandidateSchemas`, or `createIndexMetaDataRevisionCandidates`. - Tests were updated for the renamed factory and revision-candidate path (`infra/common/src/test/java/org/apache/shardingsphere/infra/metadata/database/ShardingSphereDatabaseFactoryTest.java:108-125`, `mode/core/src/test/java/org/apache/shardingsphere/mode/metadata/refresher/pushdown/type/table/CreateTablePushDownMetaDataRefresherTest.java:99-116`). ### Review Details - **Reviewed Scope:** PR #38890 latest head `e3176ed9adba29bb2097b2488d2ebac81a8314c6`; base/merge-base `2867056486e289f8db185b3fcfcae9bfb42c7178`. GitHub `/pulls/38890/files` matched the local triple-dot file list. Reviewed all 15 changed files in `infra/common`, `mode/core`, `features/encrypt/core`, and `features/sharding/core`. - **Not Reviewed Scope:** Full repository build, E2E image jobs, and runtime database/proxy scenarios. They are broader than this behavior-preserving rename. GitHub `CI`/E2E checks were still in progress at review time; `Check - Spotless`, `Check - CheckStyle`, and `Check - License` had completed successfully. - **Verification:** `./mvnw -pl infra/common,mode/core,features/encrypt/core,features/sharding/core -am -DskipITs -Dcheckstyle.skip -Dspotless.check.skip=true -Dsurefire.failIfNoSpecifiedTests=false -Dtest=ShardingSphereDatabaseFactoryTest,MetaDataReviseEngineTest,IndexReviseEngineTest,TableMetadataReviseEngineTest,SchemaMetaDataReviseEngineTest,EncryptIndexReviserTest,ShardingIndexReviserTest,MetaDataContextsFactoryTest,CreateTablePushDownMetaDataRefresherTest test` passed with exit code 0. `git diff --check` passed with exit code 0. - **Release Note / User Docs:** Not required. The PR only renames internal metadata-revision identifiers and test references; it 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]
