Skip to content

Set GitHub commit status on the built commit - #1555

Open
JuanitoFatas wants to merge 1 commit into
danger:masterfrom
JuanitoFatas:status-on-built-commit
Open

JuanitoFatas wants to merge 1 commit into
danger:masterfrom
JuanitoFatas:status-on-built-commit

Conversation

@JuanitoFatas

Copy link
Copy Markdown
Contributor

Fixes #1543.

What happened

Danger asked the GitHub API "what is the PR head?" right when it ran. Then it posted the status there.

So if you pushed commit A, then B, and the build for A started late, A's result landed on B. (And a slow build could overwrite B's real status with A's result.)

The fix

A CI source can now tell Danger which commit it built (commit_sha). Danger posts the status on that commit. If the CI source doesn't know, Danger falls back to the PR head, same as today.

  • GitHub Actions: pull_request.head.sha from the event payload. Not GITHUB_SHA and not git rev-parse HEAD. On pull_request events, both of those are the test merge commit, which isn't part of the PR. A status there never shows up on the PR.
  • Buildkite: BUILDKITE_PULL_REQUEST_HEAD_COMMIT, then BUILDKITE_COMMIT. Merge commit builds put the PR head in BUILDKITE_PULL_REQUEST_HEAD_COMMIT. I only accept a full hex SHA, because BUILDKITE_COMMIT can be the symbolic HEAD before checkout.

Only the status moves. The diff and the inline comment commit_id still use the PR head from the API, so inline comments stay lined up with the diff.

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.

GitHub commit status applied to wrong commit when multiple builds run in parallel

1 participant