DanielLeens commented on PR #12943: URL: https://github.com/apache/seatunnel/pull/12943#issuecomment-6391373554
Thanks @goutamadwant, and thanks for the quick turnaround on the earlier rounds. I do not have a new commit to review: the head is still `c931c7ffa41202a1c08f2aa48a11e473f69de017`, which is the head my latest review already covers (all four earlier blockers were resolved there). So there is nothing new to re-review until you push again. Two practical points for this head: 1. **Merge conflict.** `dev` has moved since that review (the branch is now 5 behind / 15 ahead, and GitHub reports it as conflicting). Please sync with the latest `dev` and resolve the conflicts. Since this PR touches engine code, please re-run the affected engine and Kubernetes checks after the sync, and mention in a comment if any conflict resolution touched logic rather than just textual overlap. 2. **CI.** The fork `Build` run for `c931c7f` was still showing as queued when I looked, and the apache-side `Build` pointer has no result yet. I cannot approve or start workflows myself (that needs a committer with write access, such as @davidzollo). If the fork run stays queued, please check that Actions is enabled on your fork; a push after the dev sync will also trigger a fresh run on the new head. Remaining non-blocking items from my last review (Issues 1-3: terminal coordinator status across failover plus a regression test and a note, the FAILED vs CANCELED note in `incompatible-changes.md`, and the inline FQNs in the IT code) are still welcome in the same push. The only other gate on record is @nzw921rx's `CHANGES_REQUESTED`, which will need their re-review or a maintainer dismissal once the code is final. Once the sync is pushed and CI is green on the new head, I will take another look. -- 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]
