Use ${GITHUB_SHA} instead of HEAD to avoid race conditions

[arvid220u]

Using HEAD exposes you to a race condition where if commits A and B are pushed in close succession, the breaking change check for A will not compare A with its previous commit (but instead compare B with A). Thus there would be no CI failure if commit A contains a breaking change, which is unfortunate.

Using ${GITHUB_SHA} protects against this race condition.

This approach still does not solve the problem where a single push may contain both of the commits, since GitHub Actions would only run the action on the last commit of the push. To solve that, you may want to get the first commit of the push using ${GITHUB_EVENT_PATH}, which introduces extra complexity, so I don't think it's worth adding that to the README.

Misc change: changing to use ${GITHUB_REPOSITORY} variable to make it more copy-paste friendly.

[CLAassistant]

All committers have signed the CLA.

[emcfarlane]

Thanks for raising the issue! I agree and think using the GitHub variables would be an improvement. However, I would like to propose the following changes.

${{ github.event.repository.clone_url }}#format=git,commit=${{ github.event.pull_request.base.sha || github.event.before }}

This would cover the edge cases:

• github.event.before on multiple commits refers to the commit before the push event.
• github.event.pull_request.base.sha would support running the breaking change against the main branch on a pull_request event.

This setting is the default we are using in the new buf-action.

[emcfarlane]

I think this would fail for pull_request events as the GITHUB_SHA is the last puhsed to the current branch: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
Could you validate this change works for push and pull_request event types?

[arvid220u]

Ah, didn't realize there was a new buf-action! I will close this PR in favor of switching to the new action, then. Thank you for the response and comments!