andygrove commented on code in PR #5842:
URL: https://github.com/apache/datafusion-comet/pull/5842#discussion_r3989943377
##########
.github/workflows/ci.yml:
##########
@@ -305,3 +320,61 @@ jobs:
spark-short: '4.1'
spark-full: '4.1.3'
java: 17
+
+ # ---------------------------------------------------------------------------
+ # required_checks: one flat job that aggregates every other job's result, so
+ # that `main` has a single name it can safely require.
+ #
+ # Nothing requires it yet. It is landing first, on its own, so that we can
+ # watch it report on real pull requests before `.asf.yaml` names it in
+ # `required_status_checks` for `main`.
+ #
+ # None of the jobs above can be required directly, because the name a caller
+ # of a reusable workflow publishes depends on whether it ran:
+ #
+ # skipped by `if:` one check run named exactly `PR Build (Linux)`
+ # actually ran only `PR Build (Linux) / Spark 4.1, JDK 17 [exec]`,
+ # ... and no bare `PR Build (Linux)` at all
+ #
+ # So requiring the bare name would block every code change, and requiring a
+ # nested name would block every docs-only change. Both hang rather than fail,
+ # and a required context that never reports also locks `.asf.yaml` itself,
+ # which then needs an INFRA ticket to unwedge. Aggregating into one flat job,
+ # whose name is published on every event, avoids the whole class of problem.
+ #
+ # `if: always()` is what makes this work: without it the job inherits the
+ # default `success()` and is itself skipped the moment any dependency fails.
+ # ---------------------------------------------------------------------------
+ required_checks:
+ name: Required Checks
+ if: always()
Review Comment:
Confirmed. The label run's `needs` only sees its own run, and GitHub keeps
the most recent check run per name per commit, so a `dependencies` label
landing a minute after a push would have published a green `Required Checks` on
top of a still-running or red commit run. Same mechanism as #5007.
Fixed in 8074e0174 by making the check name depend on the event:
```yaml
name: ${{ github.event.action == 'labeled' && 'Required Checks (label run)'
|| 'Required Checks' }}
```
Label runs now publish under a name nothing requires, so they can never
overwrite the commit run's verdict, and they still give the committer an honest
aggregate for the opt-in suite they asked for. Skipping the job on `labeled`
was not an option for the reason you gave: a skipped check run still carries
the name and counts as passing.
`check-ci-config.py` enforces the shape: the name must be that expression,
the `labeled` branch must differ from the fallback, the fallback is what has to
match the `.asf.yaml` context, and `.asf.yaml` may never require the label-run
name. Mutation-tested each of those (literal name, identical branches,
requiring the label-run name, renaming the fallback with a context declared)
plus the earlier cases, all caught with a specific message.
Merging `labeled` back into the PR tier was the other option and I did not
take it, because it re-creates the duplicate-pipeline cost the label policy
exists to avoid.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
