sandynz commented on PR #38723:
URL: https://github.com/apache/shardingsphere/pull/38723#issuecomment-4541049544
### Decision
- **Merge Verdict: Mergeable**
- **Reviewed Scope:** Latest commit `eff10cf6fb9`;
`scripts/smart-antlr-rebuild.sh` state parsing, baseline selection, state
writing, `--force` option, and Maven invocation path.
- **Not Reviewed Scope:** Remote CI results; a real full ANTLR rebuild
across all grammar modules; generated files under `target/`.
- **Need Expert Review:** No.
### Basis
- The commit directly addresses the root cause: the previous single-line
`.antlr_last_commit` format could only remember one branch and forced full
rebuilds after switching branches.
- The new state model separates `active:<branch>:<commit>` for the
generated-output baseline from `branch:<branch>:<commit>` records for branch
progress, which preserves master and feature branch state without relying
only on branch-name equality (`scripts/smart-antlr-rebuild.sh:40`,
`scripts/smart-antlr-rebuild.sh:95`, `scripts/smart-antlr-rebuild.sh:140`).
- Backward compatibility is covered: old `branch:commit` state is still
parsed and migrated through the same write path
(`scripts/smart-antlr-rebuild.sh:109`, `scripts/smart-antlr-rebuild.sh:157`).
- The `--force` option provides the requested from-scratch rebuild path
without changing the default incremental workflow
(`scripts/smart-antlr-rebuild.sh:49`, `scripts/smart-antlr-rebuild.sh:210`).
- The Maven compile path remains scoped and still uses `-am`, so dependent
modules are included when affected modules are rebuilt
(`scripts/smart-antlr-rebuild.sh:258`).
- Risk scan:
- **Design:** The change is localized to the script state model and does
not alter parser grammar, runtime routing, metadata, SPI, JDBC, or Proxy
behavior.
- **Performance:** State parsing is linear in the small state file;
module scanning remains only for force/no-baseline paths. No hot runtime path
is touched.
- **Compatibility:** Existing single-line state files remain accepted.
Git branch names cannot contain `:`, so the colon-delimited format is
compatible with valid branch names.
- **Regression:** The branch-switch-back counterexample validates that
switching from feature back to master rebuilds the affected module and
preserves both branch records.
- Adversarial pass:
- **Adjacent-feature regression path:** Multi-branch ANTLR module
selection was tested with old master state, feature branch rebuild, no-change
skip, force rebuild, and switch-back-to-master rebuild.
- **Config-disabled / feature-flag-off path:** There is no runtime
feature flag; the default incremental path and explicit `--force` path were
both validated.
- **Original symptom path:** The old single-record overwrite problem is
covered by isolated migration and multi-branch state preservation checks.
### Pre-Merge Checks
- `bash -n scripts/smart-antlr-rebuild.sh && git diff --check HEAD~1..HEAD
-- scripts/smart-antlr-rebuild.sh && bash scripts/smart-antlr-rebuild.sh --help
>/tmp/smart-antlr-help.out` passed with exit code `0`.
- Isolated behavior test passed with exit code `0`: old-format state
migration, feature branch rebuild, no-change skip, and `--force` full-scan path.
- Switch-back counterexample test passed with exit code `0`: after
rebuilding on `feature`, switching back to `master` selected the affected
module and preserved both branch records.
- `./mvnw spotless:apply -Pcheck -T1C` passed with exit code `0` and
`BUILD SUCCESS`.
- `./mvnw checkstyle:check -Pcheck -T1C` passed with exit code `0` and
`BUILD SUCCESS`.
- Working tree remained clean after review verification.
--
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]
