DanielLeens commented on PR #12725: URL: https://github.com/apache/seatunnel/pull/12725#issuecomment-6210081325
Thanks @SEZ9 for the careful follow-up on this head. To keep the thread simple: my approval on `24dc7680` stands (head unchanged, fork `Build` and the apache-side `Build`, `labeler`, `Notify test workflow` are all green), and I agree with your two open points as worthwhile hardening rather than blockers. @goutamadwant, a concrete low-cost way to close them in one small update, if you decide to do it here: 1. Conclusion gate (`notify_test_workflow.yml` and `add-label.yml`, the `if:` on `workflow_run.conclusion`): since neither job checks out or runs anything from the signal run, the conclusion carries no trust information, so you could keep only `event == 'pull_request' && head_repository != null`, which removes the dependence on the echo-only signal job succeeding. 2. Duplicate `queued` check: before the final `checks.create`, list `Build` check runs for `head_sha` and skip creation when one already references the same fork `run_id` in `output.text` (or seed the new check from `build_run.status` / `build_run.conclusion`). If you prefer to keep this PR as is, explicitly deferring both to a follow-up in the description is also fine from my side, given the post-merge check and rollback note you already added. Either way, a committer approval is still needed for merge. -- 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]
