SEZ9 commented on PR #12824: URL: https://github.com/apache/seatunnel/pull/12824#issuecomment-6553239010
Thanks for keeping this migration moving and for resolving the `dev` conflict — the automated review confirms the conflict is cleared and the `Build` check on the current head is green, and it found no new functional blocker after re-reading the full diff and the runtime callers (FactoryUtil, SeaTunnelConfValidateCommand, the REST/CLI exporters via OptionRulesService and SeaTunnelMetadataExporter). The semantic change is also clearly documented in the thread: `A.and(B).or(C)` now evaluates and renders as `(A && B) || C`, single-condition and pure-OR builder chains are unchanged, and only hand-built mixed chains intentionally change behavior. That matches what I'd expect from dropping the duplicate `Expression` wrapper in favor of `Condition` chains. Remaining asks before this can be merged: 1. **Add the two short Javadoc contracts** that were called out as "Issue 1" in the earlier source review. This is the only outstanding item and has been unchanged across the follow-ups; it should be a small docs-only commit. If it's unclear which two spots are meant, please say so and I'll point to them explicitly rather than guess. 2. Once that commit is pushed, please drop a note here so I can re-run a quick look at the new head; a write-capable committer approval is the last gate after that. Nothing else from my side — nice, careful work on the migration. <!-- streview-comment:2196 --> -- 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]
