terrymanu commented on PR #39886: URL: https://github.com/apache/shardingsphere/pull/39886#issuecomment-5727834804
### Result **Review Result: Mergeable** The local candidate correctly introduces standard Bean Validation across the rule configuration object graph and invokes it during rule construction, dynamic configuration refresh, and rule-item updates. No blocking compatibility, ownership, dependency, or correctness issues were found. ### Evidence - Root configurations, nested configurations, container elements, and changed rule items are validated through the same Apache BVal `Validator`. - SQL Parser cache capacities use non-negative constraints, preserving Caffeine’s existing support for zero capacity. SQL Federation retains its existing strictly positive requirement. - Optional Timestamp Service and Transaction properties were not incorrectly made mandatory. - Modules using validation annotations declare the Validation API directly, while the BVal implementation remains centralized in the shared runtime modules. Dependency analysis confirmed `jakarta.validation-api:2.0.2` and `bval-jsr:2.0.6`, with no Hibernate Validator. - Seventeen relevant test classes passed, covering cascading validation, validation before empty-configuration filtering, dynamic refresh, and validation before rule-item mutation. - Exact-scope Spotless and Checkstyle checks passed for all 67 task files with exit code 0. - The full-repository Apache RAT check passed with exit code 0. ### Coverage - Candidate type: standalone local candidate. - Baseline: `fdd58d899c30e92e02edd947b5a5ddaedc64da6f`. - Attributed local delta: 67 files, comprising 13 Maven POMs, 37 production Java files, and 17 test files. The pre-existing `.codex/skills/review-pr/scripts/__pycache__/` directory was excluded. - Reviewed behavior clusters: validation entry points, rule configuration object graphs, Validation API/BVal dependencies and distribution, and regression tests. - Completed review lenses: root cause and behavior, blast radius and contracts, tests and runtime behavior, and final convergence. - Unresolved evidence gaps: none. - This was a code-correctness review. Remote CI was not reviewed; the Spotless, Checkstyle, test, dependency, and RAT results are local verification of the current candidate. -- 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]
