Skip to content

fix: repair clang-format CI for fork PRs and push events#2731

Merged
ibell merged 1 commit into
masterfrom
fix/issue2724-clangformat-ci
Apr 17, 2026
Merged

fix: repair clang-format CI for fork PRs and push events#2731
ibell merged 1 commit into
masterfrom
fix/issue2724-clangformat-ci

Conversation

@ibell
Copy link
Copy Markdown
Contributor

@ibell ibell commented Apr 17, 2026

Summary

Fixes #2724 — the dev_clangformat workflow was silently passing on the majority of PRs.

  • Fork PRs: replaced remotes/origin/$GITHUB_HEAD_REF / remotes/origin/$GITHUB_BASE_REF with commit SHAs from the GitHub Actions context (github.event.pull_request.base.sha and github.sha). The fork's branch is never fetched into origin, so the old ref always resolved to nothing and git diff reported 0 changed files.
  • Push events: GITHUB_HEAD_REF and GITHUB_BASE_REF are only populated for pull_request events, not push. The fix detects the event type and uses github.event.before / github.sha for push events.
  • Action versions: bumped actions/checkout v3 → v6 and actions/upload-artifact v4 → v7 (both at latest stable).
  • Removed redundant git fetch --all (not needed since fetch-depth: 0 already fetches everything and we now use SHAs).

Test plan

  • Open a PR from a fork branch and confirm the workflow now correctly identifies changed .cpp/.h files
  • Verify a push to master/develop runs the diff against github.event.before
  • Confirm a correctly-formatted PR still passes (exit 0)
  • Confirm a poorly-formatted PR fails and uploads the patch artifact

🤖 Generated with Claude Code

- Use commit SHAs from GitHub context instead of remotes/origin/<branch>
  names, which don't exist for PRs from forks
- Handle push events separately (GITHUB_HEAD/BASE_REF are empty on push)
- Bump actions/checkout v3 -> v6, actions/upload-artifact v4 -> v7
- Remove redundant `git fetch --all`

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@henningjp henningjp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Verified PR behavior from forked repos using #2732 draft PR.

  • Looked like it passed (no artifacts generated)

So, I intentionally messed with the code format in another commit.

  • It successfully failed the clang-format test.
  • It successfully uploaded the clang-format repair artifact.

I don't know how to check the second Test Plan item from master/develop branch.

@ibell ibell merged commit 7a394ac into master Apr 17, 2026
1 check passed
@ibell ibell deleted the fix/issue2724-clangformat-ci branch April 17, 2026 22:03
@ibell ibell added this to the v8.0.0 milestone May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ISSUE] Clang-format CI Misbehaving (silently) on Remote PRs

2 participants