andygrove opened a new pull request, #5842:
URL: https://github.com/apache/datafusion-comet/pull/5842
## Which issue does this PR close?
Part of #5838.
## Rationale for this change
`main` requires no status checks today, only one approving review. Before we
can add one — which #5838 needs, since a merge queue only ever waits on
required checks — we need a name that is actually safe to require, and right
now we don't have one.
Every heavy job in `ci.yml` is a thin caller of a reusable workflow, and the
check name a caller publishes depends on whether it ran:
| Caller state | Check runs published
|
| ---------------- |
-----------------------------------------------------------------------------------------------------
|
| skipped by `if:` | one run named exactly `PR Build (Linux)`, conclusion
`skipped` |
| ran | only `PR Build (Linux) / Spark 4.1, JDK 17 [exec]` and
friends, and no bare `PR Build (Linux)` at all |
I confirmed both against real commits — bdd2aeb (md-only, everything
skipped) and 36caf8e (heavy jobs ran). There's no name that reports in both
cases, so requiring the bare name would block every code change, and requiring
a nested name would block every docs-only change. Worse, both of those hang
waiting for a check that never arrives rather than failing outright, and a
required context that never reports also blocks the merge that would fix
`.asf.yaml` — at that point only INFRA can remove the check by hand.
apache/datafusion ran into the same class of problem in apache/datafusion#17538
and had to revert it in apache/datafusion#17629.
## What changes are included in this PR?
A `required_checks` job at the bottom of `ci.yml` that `needs:` every other
job and publishes a single flat `Required Checks` context on every event. It
runs `if: always()` and treats `skipped` as a pass, so it only goes red when an
upstream job reports `failure` or `cancelled`.
Nothing requires it yet, and that's deliberate. It lands on its own so we
can watch it report on real pull requests before `.asf.yaml` names it, since
that's the step that's expensive to get wrong. The follow-up PR does that and
turns the queue on.
`dev/ci/check-ci-config.py` also gains two invariants, in the same spirit as
the ones already in there: every `ci.yml` job except `docs` has to appear in
`required_checks.needs`, and once `.asf.yaml` does declare a required context,
the job's `name:` has to keep matching it. Both sides of that pair are silent
when broken.
## How are these changes tested?
`preflight` already runs `check-ci-config.py`, so the new invariants gate
every PR from here on. I mutation-tested all four failure modes against a clean
baseline — dropping a job from `needs`, renaming the aggregator job, typoing
the context in `.asf.yaml`, and adding a new heavy job that forgets to register
— and each one is caught with a specific message.
`actionlint` and `prettier --check "**/*.md"` are clean.
The job is also observable on this PR itself: `Required Checks` should show
up in the checks list and be green.
--
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]
