DanielLeens commented on PR #12129: URL: https://github.com/apache/seatunnel/pull/12129#issuecomment-5555751375
Thanks for the detailed follow-up — this is exactly the kind of engagement that makes a hardening PR pleasant to review. **On CI:** your read is right, and I double-checked it against the actual `Build` check-run output on the current head — it's the classic "Unable to detect the workflow run... please enable GitHub Actions in your forked repository" signature, not a code problem. Nothing you need to change here; once Actions are enabled on your fork and you push the retrigger commit, it should produce a real run. Go ahead and ping this thread when that's done and I'll take a fresh look right away. **On the `PhysicalPlan`-level mock test:** your call to skip it is the right one, and I'm genuinely fine leaving it out. My original note flagged it explicitly as non-blocking, and your reasoning is sound — a test that has to reach through `updateStateInfo`/`reportJobStateEvent`/`stateProcess()`/`JobMaster`/`EngineConfig` just to assert `EnumSet` membership would mostly be validating its own mock wiring rather than real behavior, and that fragility risk is real. Combined with the fact that the change is already provably behavior-preserving against the actual enum ordinals, keeping this PR narrowly scoped is the better tradeoff — no need to add it now, and it's fine as a possible follow-up later or not at all. Please don't feel any obligation on that point. So the only open item is CI actually running. Once that's green I'll do a fresh pass and we can move this toward merge. Thanks again for a well-reasoned, first-contribution PR — nice work tracing every ordinal usage site back to the source. -- 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]
