terrymanu commented on PR #40200:
URL: https://github.com/apache/shardingsphere/pull/40200#issuecomment-6251322574
### Result
Review Result: Mergeable
The code-correctness review passed with no blocking issues.
### Evidence
- Filtering null values before joining emitted properties fixes dangling
commas when maxPoolSize is omitted or only custom properties are present, while
preserving property order.
- The previous PropertiesBuilder and helper-ordering issues are fixed.
- Dependency compilation from the same commit passed, exit 0.
- ConvertYamlConfigurationExecutorTest: 17 passed, 0 failures, 0 errors, 0
skipped, exit 0.
- Checkstyle and Spotless passed, exit 0. Reports confirm coverage of both
changed Java files.
### Coverage
- Candidate: Public PR, with no local code delta.
- Head: 5a902a3561bc06fa3973696d98b626d22a07bf68
- Base / merge-base: 01446d4fad48450e7fc3eb3b3631b25d6bf411df
- All 3 changed files reviewed:
- RELEASE-NOTES.md
-
infra/distsql-handler/src/main/java/org/apache/shardingsphere/distsql/handler/executor/configuration/convert/ConvertYamlConfigurationExecutor.java
-
proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/distsql/ral/queryable/yaml/ConvertYamlConfigurationExecutorTest.java
- Reviewed against Issue #40198, the existing conversion contract, and
repository standards. Completed root-cause, contract-impact, test/runtime-path,
and convergence checks across property rendering, regression tests, and release
notes. No new API, SPI, or project support commitment is introduced.
- Two independent review rounds completed. The first-round disagreement
about line-wrapping rule applicability was resolved through substantive review;
the additional round agreed. All four ledgers and session validation passed,
with no unresolved evidence gaps.
- Local style command:
`./mvnw -B -pl infra/distsql-handler,proxy/backend/core
-DspotlessFiles="$S" -Dcheckstyle.includes="$C"
-Dcheckstyle.includeResources=false -Dcheckstyle.includeTestResources=false
spotless:check checkstyle:check -Pcheck -T1C`
- `$S` contains exact path expressions for the two Java files above; `$C`
contains their source-root-relative paths. Both checks covered both files, exit
0.
- RELEASE-NOTES.md: Both checks are Not Applicable under the parent POM’s
formatting scope and Checkstyle’s configured file extensions.
- This result covers code correctness only; CI was not queried.
- Only the authorized single fetch was performed. No branch switching or
code modification was performed. Temporary review materials were cleaned up.
--
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]
