chore: Skip Code CI for non code changes#22894
Conversation
Yes! you cannot path-ignore for MQ because all required steps have to be green |
|
A cool thing to test would be: how many non-code changes, e.g. configs, caused test failures in the last N months? I wanted to do that myself, but my internet is horrible for the next couple of days. Also, the approach looks correct to me, but with those mandatory steps, 50% of the time I end up predicting it wrong and accidentally blocking merging for everyone. If you like, feel free to push it to https://github.com/apache/datafusion-sandbox/ to make sure everything is passing. Also happy to do that testing myself in a few days |
Thanks for the hint, its actually nice to have ASF sandbox, I had to experiment with fork-subfork when was engaging in assessing user fork CI |
| name: cargo test doc (amd64) | ||
| needs: linux-build-lib | ||
| needs: [linux-build-lib, detect-changes] | ||
| if: needs.detect-changes.outputs.has_code == 'true' |
There was a problem hiding this comment.
i think we run doc tests against the code in the md files too?
e.g.
datafusion/datafusion/core/src/lib.rs
Lines 934 to 940 in b8998c7
| name: check configs.md and ***_functions.md is up-to-date | ||
| needs: linux-build-lib | ||
| needs: [linux-build-lib, detect-changes] | ||
| if: needs.detect-changes.outputs.has_code == 'true' |
There was a problem hiding this comment.
could this mean if configs.md is manually changed without a code update we could miss this? (or was that an existing issue?)
|
|
||
| on: | ||
| pull_request: | ||
| paths-ignore: |
There was a problem hiding this comment.
maybe we keep this check even for doc files?
Which issue does this PR close?
Rationale for this change
Problem. Doc-only PRs (a typo fix in
README.md, a release-note edit) trigger the full ~25-job Rust CI matrix inrust.ymlanddependencies.yml. Wasted runner time and queue capacity forchanges that can't break code.
Solution. Adopt Apache Spark's gating pattern (
spark/.github/workflows/build_and_test.yml): a single change-detection job, then per-jobif:.rust.ymlanddependencies.ymleach gain adetect-changesjob that diffs the PR against anIGNORE_CODE_CI_FOR_PATHSglob list and emitshas_code.needs: detect-changesandif: needs.detect-changes.outputs.has_code == 'true'.skipped(which GitHub branch protection treats as satisfying required status checks).Why per-job
if:and not workflow-levelpaths-ignore. The repo'srequired_status_checkslist in.asf.yamlis incompatible withpaths-ignore—ci/scripts/check_asf_yaml_status_checks.pyexplicitly rejects workflows that have it, because a workflow-level skip never reports a status, leaving required checks in "Expected" and blocking merge. A job-level
if: falsedoes report(
conclusion: skipped), which is what unblocks doc-only PRs without weakening protection on code PRs.Other workflows.
codeql.yml,breaking_changes_detector.yml,large_files.ymlkeep workflow-levelpaths-ignoresince none are required checks.extended.yml,audit.yml,docs_pr.yamlwerealready path-filtered to the right files.
.asf.yaml. One added context:"Detect changes". The detector script is fail-open (any error →has_code=true→ run full CI), so a transient git failure can't silently merge a broken PR; thisrequired check makes a broken detector itself blocking