terrymanu commented on PR #39006:
URL: https://github.com/apache/shardingsphere/pull/39006#issuecomment-4885814851
### Summary
**Review Result: Mergeable**
Reason: Code-scope review only; CI was not reviewed by request. The latest
PR head removes the proxy compute-node label feature consistently across
runtime model, cluster persistence, DistSQL parser/executors, SPI/native
metadata, tests, E2E datasets, and user/management documentation. No code-level
blocker remains in the reviewed scope.
### Evidence
- The removed label capability has no remaining in-scope command, parser
statement, executor, node-path, handler, persistence, or documented
registry-path references in the changed files.
- `SHOW COMPUTE NODE INFO` and `SHOW COMPUTE NODES` docs no longer advertise
the removed `labels` result column.
- The management reference docs no longer list the deleted
`nodes/compute_nodes/labels` governance path.
- The four changed `empty_rules` RAL E2E datasets have matching
metadata/value counts: `7=7`, `9=9`, `7=7`, `9=9`.
- Focused tests passed for the touched behavior paths:
`ComputeNodeInstanceTest`, `ComputeNodeInstanceContextTest`,
`ClusterComputeNodePersistServiceTest`, `ShowComputeNodeInfoExecutorTest`,
`ShowComputeNodesExecutorTest`, and `BootstrapInitializerTest`.
- Repository formatting/style gates passed: `spotless:apply` and
`checkstyle:check`.
### Review Details
- Review Focus: Code Correctness Review. CI not reviewed by request.
- Reviewed Scope: PR #39006 latest head
`084f6950f88f7242f699497464a83201393b7fad`, base `master` /
`fffe6a3bec751a8434f95cb8bc51e966b69fb1cc`, local merge-base
`fffe6a3bec751a8434f95cb8bc51e966b69fb1cc`. GitHub `/pulls/39006/files`
returned 65 files and matched the local triple-dot scope.
- Reviewed areas: proxy label removal across instance model, context-manager
construction, cluster persistence/event handling, parser
grammar/statements/fixtures, proxy config/executors/SPI, native reachability
metadata, deleted LABEL/UNLABEL docs, updated SHOW docs, management reference
docs, and RAL E2E datasets.
- Not Reviewed Scope: GitHub Actions status/logs and full Docker E2E runtime
were not reviewed in this code-correctness pass. Unrelated uses of “labels” for
metrics, result-set column labels, PostgreSQL security labels, issue labels,
and MCP response labels were treated as out of scope.
- Verification:
- PR metadata and changed-file scope check: exit `0`; confirmed latest
head and 65-file scope.
- Changed-file residual search for removed proxy label
commands/classes/node paths/docs: exit `1`; no matches.
- E2E dataset column/value count guard: exit `0`; all four changed
datasets matched.
- Focused Maven tests: exit `0`; `BUILD SUCCESS`.
- `./mvnw spotless:apply -Pcheck -T1C`: exit `0`; `BUILD SUCCESS`.
- `./mvnw checkstyle:check -Pcheck -T1C`: exit `0`; `BUILD SUCCESS`.
- Release Note / User Docs: User docs and management reference docs were
required and verified. No additional release-note blocker was found for this
code-correctness review.
--
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]
