Skip to content

Conversation

@rneatherway
Copy link
Contributor

As we move towards analysing the merge commit for pull requests by
default, we should stop sending /refs/pull/n/head rather than
refs/pull/n/merge unless the checked-out SHA has actually changed.
Here we assume that any change (compared to GITHUB_SHA) indicates that
git checkout HEAD^2 has been run earlier. This may sometimes be
incorrect (e.g. git checkout mybranch), but in that case the ref
would be wrong either way.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

As we move towards analysing the merge commit for pull requests by
default, we should stop sending `/refs/pull/n/head` rather than
`refs/pull/n/merge` _unless_ the checked-out SHA has actually changed.
Here we assume that any change (compared to GITHUB_SHA) indicates that
`git checkout HEAD^2` has been run earlier. This may sometimes be
incorrect (e.g. `git checkout mybranch`), but in that case the ref
would be wrong either way.
@rneatherway rneatherway force-pushed the rneatherway/optional-merge branch from 74587c5 to 7795860 Compare September 17, 2020 12:11
@robertbrignull robertbrignull self-assigned this Sep 18, 2020
Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@chrisgavin chrisgavin left a comment

Choose a reason for hiding this comment

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

I've tested this with the backend changes required to map merge commit annotations to the head commit and it worked great!

@rneatherway
Copy link
Contributor Author

Thanks!

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.

4 participants