michaelneale Sure, something is going forward to

It would be great to see this also added to the bitbucket-branch-source plugin.

Yes, nickbrown. We will make sure to include this in BB as well. Our goal is to have as much parity between GH and BB as possible. My first comment was to that effect. amuniz

Are there any plans to allow users to opt out of building PR merge refs? I don't want to be doing builds for revisions that will never exist. This is a serious problem for me, to the point that I'm about to make a custom build with +refs/pull/*/head instead of +refs/pull/*/merge in GitHubSCMSource.java.

The original description of this seems to mix the issues at hand. One issue is that origin PRs should be allowed, via an option. The second issue, completely unrelated, is that any PR (from remote or origin) should expose options to build either or both of HEAD and MERGE.

Well, they are related in that all reasonable options about what to build and based on what reference ought to be offered.

Tricky because the GitHub API documentation is frustratingly vague and incomplete. When getting a pull request, there is an undocumented .base.sha which seems to be the common ancestor between the PR branch and the base branch—very different from pull/*/merge and merge_commit_sha, which both seem to represent a virtual merge of the PR head onto the current base branch head (or, rather, a time-delayed snapshot thereof, and as noted in JENKINS-33237 not a very reliable one).

Rough outline of changes:

• UntrustedPullRequestSCMRevision needs to be generalized to a PullRequestSCMRevision which always records a baseHash, plus a trusted field, so that all PullRequestSCMHead will use this kind of revision. Implies a readResolve for settings compatibility.
• GitHubBranchSource (and, by extension, GitHubSCMNavigator) needs to be able to be configured as to whether it will create jobs for:
  • an origin branch with no corresponding PR
  • an origin branch with a corresponding PR (but built as a branch: named by branch name, not sending status notifications, not offering to merge with a base branch)
  • a PR from an origin branch
  • a PR from a fork
• For the latter two, we additionally need to be able to decide whether to build:
  • The PR head. Means the trusted revision, if applicable, is the .base.sha, and no merge is required.
  • The merge into the current base branch. Means base branch updates need to trigger fresh PR builds (SCMRevision.equals should be false when the base branch has moved forward). The trusted revision is the current base branch head. merge_commit_sha is deprecated and might be deleted in the future; pull/*/merge is buggy; so the plugin rather needs to explicitly look up and record the current base branch head, and perform an explicit merge (probably via decorateRevisionToBuild/PreBuildMerge, or less obviously via getCandidateRevisions).
• getRefSpecs need not refer to PR refs, since build would anyway be overridden for PRs.
• The two overloads of doRetrieve need to produce consistent SCMRevision implementations for PRs. Currently we return the PR head during branch indexing, but the virtual merge commit later, which violates the API contract and intuition.

TBD whether it is necessary to allow a PR to be built both as head and as merge. I think I have heard this request. In such a case, two jobs need to be created for it with different names, perhaps PR-123 vs. PR-123-merge.

jglick thanks for the writeup. I'd just like to confirm the request that it should be possible to build the PR head or merge – that is, I know some have a preference for the merge (current behavior) but it should also be possible to do the head instead. I don't know that a user would want to build both refs, but definitely being able to choose one or the other is valuable.

I'd also like to say that it's still valuable to provide github status notifications on any ref, not just pull requests. The information is accessible through the API and very useful. On a related note, being able to control the status context is also useful.

Very thorough writeup.
I think the default of building merge for PRs is consistent with forked pull requests.

confirm the request that it should be possible to build the PR head or merge

Yes, I would consider that a requirement, and making it possible to build both does not seem like any extra work. What the best way to present all these options in the UI would be, I am not sure, but I will leave that to someone with a good eye for it.

valuable to provide github status notifications on any ref, not just pull requests

True, currently most notifications do get sent on all refs. GitHub will only display status on commits in the PR UI, but there is no harm in always sending it.

being able to control the status context is also useful

There are certainly use cases for displaying customized statuses but it seems off topic here.