ci: improve gate job diagnostics and document branch protection requirement#465
ci: improve gate job diagnostics and document branch protection requirement#465staging-devin-ai-integration[bot] wants to merge 1 commit into
Conversation
…rement The all-checks gate job already correctly handles skipped sub-workflows (only fails on failure/cancelled, allowing skipped results), but its error output didn't identify which job failed. Changes: - Log each job name alongside its result for easier debugging - Add comment documenting that all-checks should be the ONLY required status check in branch protection, so skipped path-filtered jobs never block merge - Remove the generic success message (the job passing is sufficient) Signed-off-by: Staging-Devin AI <166158716+staging-devin-ai-integration[bot]@users.noreply.github.com> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![staging-devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/870844?s=60&v=4){kind=small}
There was a problem hiding this comment.
📝 Info: Workflow license headers are covered by REUSE annotations
Although .github/workflows/ci.yml does not contain an inline SPDX header, this PR did not add a new file and the repository-level REUSE.toml annotates all **/*.yml/**/*.yaml configuration files with StreamKit's MPL-2.0 licensing metadata (REUSE.toml:40-56). Because of that existing aggregate annotation, I did not treat the missing inline header as a CONTRIBUTING.md SPDX violation.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| if [[ "$r" == "failure" || "$r" == "cancelled" ]]; then | ||
| echo "::error::CI check failed or was cancelled ($r)" | ||
| exit 1 | ||
| echo "::error::${names[$i]}: $r" | ||
| failed=1 |
There was a problem hiding this comment.
📝 Info: Gate still ignores skipped dependencies by design
The updated gate now prints each dependency result before deciding whether to fail, but it still only treats failure and cancelled as blocking states. That preserves the previous semantics from the base version and matches the added branch-protection comment: path-filtered reusable workflows can be skipped without blocking the single required all-checks status. I did not flag this as a bug because changes and reuse are unconditional, while skit, ui, plugins, and e2e are explicitly guarded by path filters in .github/workflows/ci.yml:57-99, so accepting skipped is intentional rather than an accidental bypass.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| failed=0 | ||
| for i in "${!names[@]}"; do | ||
| r="${results[$i]}" | ||
| printf ' %-10s %s\n' "${names[$i]}" "$r" | ||
| if [[ "$r" == "failure" || "$r" == "cancelled" ]]; then | ||
| echo "::error::CI check failed or was cancelled ($r)" | ||
| exit 1 | ||
| echo "::error::${names[$i]}: $r" | ||
| failed=1 | ||
| fi | ||
| done | ||
| echo "All checks passed or were appropriately skipped." | ||
|
|
||
| if [[ "$failed" -ne 0 ]]; then | ||
| exit 1 |
There was a problem hiding this comment.
📝 Info: Accumulating failures changes reporting, not pass/fail behavior
The old loop exited on the first failed/cancelled dependency; the new loop records failed=1, continues printing all dependency statuses, and exits once after the loop. This is a semantic change in log output only: any dependency result of failure or cancelled still makes all-checks exit nonzero, while all-success/all-skipped-where-allowed runs still pass. I therefore did not report this as a behavior bug.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #465 +/- ##
=======================================
Coverage 65.91% 65.91%
=======================================
Files 217 217
Lines 57529 57529
Branches 1597 1680 +83
=======================================
Hits 37921 37921
Misses 19602 19602
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
1 participant
Summary
Improves the
all-checksgate job inci.ymlwith better diagnostic output and documents the branch protection requirement.Investigation results: The gate job already correctly handles skipped sub-workflows — it only fails on
failureorcancelled, allowingskippedresults to pass through. Confirmed by checking recent merged PRs (e.g. #456 with 4 skipped sub-workflows, #460/#461/#462 with 1 skipped each).All Checks Passedis configured as a required status check and merges are not blocked by skipped jobs.Changes:
all-checksshould be the only required status check in branch protection — individual sub-workflow job names must not be required, as path-filtered skips would leave them permanently pendingReview & Testing Checklist for Human
mainonly requireAll Checks Passed(no individual sub-workflow job names likeSkit / LintorUI / Lint, Test & Build).github/, so thecipath filter triggers all sub-workflows)Notes
This PR is CI-only — no production code changes. The gate job logic (allow
success/skipped, fail onfailure/cancelled) is unchanged; only diagnostics and documentation are added.Link to Devin session: https://staging.itsdev.in/sessions/68824382a1da4a05bb320d129a89f155
Requested by: @streamer45
Devin Review
60addb7