ym0506 commented on PR #38685: URL: https://github.com/apache/shardingsphere/pull/38685#issuecomment-4540896479
> Merge Verdict: Not Mergeable > > Reviewed Scope: PR #38685 latest head [3905e54](https://github.com/apache/shardingsphere/commit/3905e5404658fbc0eb985c5ef6c050eec7ff96e2), 3 changed files: standalone metadata unit test and RDL E2E XML fixtures. > > Not Reviewed Scope: I did not run Maven tests locally; I relied on PR CI plus diff/source inspection. > > Need Expert Review: No special security/parser/concurrency review needed for this test-only PR. > > Positive feedback: the added coverage is aimed at the right path. It covers named rule item deletion through DatabaseRuleItem("named", "foo_rule"), and the RDL case exercises CREATE -> DROP -> CREATE -> SHOW for the same encrypt rule name, matching issue #38657’s stale runtime metadata symptom. > > Newly Introduced Issues > > [P1] Diff check fails on trailing whitespace > > Symptom: git diff --check [6d761e5](https://github.com/apache/shardingsphere/commit/6d761e53884e37bff7af1d2047221088565bbbdc)...apache/pr/38685 exits 2. > > Evidence: mode/type/standalone/core/src/test/java/org/apache/shardingsphere/mode/manager/standalone/persist/service/StandaloneMetaDataManagerPersistServiceTest.java:215: trailing whitespace. > > Risk: this conflicts with the project submission gate in CODE_OF_CONDUCT.md (line 17), especially the requirements that coding standards and build/style steps complete successfully. > > Recommended action: please remove the trailing whitespace and rerun git diff --check. > > Next Steps > > Remove the whitespace-only issue in StandaloneMetaDataManagerPersistServiceTest.java. > > Rerun: > > git diff --check > > xmllint --noout test/e2e/sql/src/test/resources/cases/rdl/e2e-rdl-create.xml test/e2e/sql/src/test/resources/cases/rdl/dataset/distsql_rdl/create_recreated_encrypt_rule.xml > > the two scoped Maven commands listed in the PR, preferably with -am. Adjusted `StandaloneMetaDataManagerPersistServiceTest` so the existing `assertRemoveRuleConfigurationItem()` test covers both unique and named database rule items. This avoids the added separator-line conflict while preserving the original coverage and keeping the `DatabaseRuleItem("named", "foo_rule")` regression check. Verified locally: - `git diff --check 6d761e53884e37bff7af1d2047221088565bbbdc...HEAD` - `./mvnw spotless:check -Pcheck -T1C` - `./mvnw checkstyle:check -Pcheck -T1C` - `./mvnw -pl mode/type/standalone/core -am ... -Dtest=StandaloneMetaDataManagerPersistServiceTest test` - `./mvnw -pl features/encrypt/distsql/handler -am ... -Dtest=DropEncryptRuleExecutorTest,CreateEncryptRuleExecutorTest test` GitHub checks are green, including `Check - Spotless`, `Check - License`, `Check - CheckStyle`, `CI`, `E2E - SQL (Smoke)`, and `E2E - SQL (Stage 2)`. -- 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]
