terrymanu commented on PR #40056: URL: https://github.com/apache/shardingsphere/pull/40056#issuecomment-5989713560
### Result **Review Result: Mergeable** The complete migration and rename follow the approved design. No blocking issues were found. ### Evidence - Default-group validation preserves the first conflicting database, all duplicate names within that database, and the original message, error code 10104, and SQLState 42S01 of `DuplicateStorageUnitException`. Validation still runs before port checks and connection pool creation. - `ProxyConfigurationLoadResult` retains the original fields and constructor structure. The independent defensive-code review found no unjustified fallback logic or additional abstractions. - All 29 tests passed. Bootstrap, E2E, and Native consumers compiled against the current source. The two new validators cover 32/32 lines and 6/6 branches; all changed Loader lines are covered. - Spotless and Checkstyle covered all 16 Java files, and RAT passed. The JAR contains no obsolete types, and the packaged Native metadata matches the source. - The original migration increased the first Loader invocation by approximately 75–83 ms and retained memory at that phase by approximately 0.9 MB, within the accepted validator initialization cost. Full Proxy startup cost was not measured. This rename adds no runtime logic. ### Coverage Standalone local code-correctness review against baseline `0163d9d7930feb1f0d982c767c0d731f84c6a34d`, covering all 20 task paths: 6 additions, 11 modifications, and 3 deletions. Coverage includes validation, exception conversion, startup consumers, and Native registrations. Root-cause, impact, testing, and runtime checks were completed, followed by final convergence. The review ledger passed validation. The R15-C scope scanner has subtask-scope and baseline-format limitations. Every path and hunk was manually checked against the original task scope. No task-owned source changes fall outside that scope, and no unresolved blocking gaps remain. Removing and renaming the old public Java types has an authorized compatibility impact. Native verification covered static metadata, packaging, and JVM compilation; no Native executable was run. E2E tests were not run. The conclusion is limited to the local code scope; remote CI was not reviewed. -- 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]
