Pull Request to scan on merged branch not head

[bradleyfalzon]

Currently, when scanning a Pull Request, GopherCI will perform the scans on HEAD. But it'd be more correct to do the scans on the merged HEAD.

If we do the scans/analysis on the merged head we'll have a higher chance of catching issues that would only occur once merged - such as the PR branched from an older copy of the base branch which has since been updated.

1. master: create branch pr1
2. master: commit and push change changing type from int32 to int
3. pr1: use the type in a function, where it's legal to use int32 but not int, such as binary.Write
4. Send in PR

GopherCI would check HEAD, which is using type int, which is OK, but on merge the type would be an int32, and therefore would be illegal.

It's still possible we could miss this case if steps 2 and 3 were swapped around.

We'd also need to consider checking each PR every time base changes, which would be quite expensive, and handling cases where there are merge conflicts (perhaps just not scanning if there's a conflict).

Thanks to @judwhite and @dominikh for highlighting this issue.

[dmitshur]

Currently, when scanning a Pull Request, GopherCI will perform the scans on HEAD. But it'd be more correct to do the scans on the merged HEAD.

This reminds me of a relevant article https://developer.atlassian.com/blog/2015/01/a-better-pull-request/. I'd recommend being familiar with it, and the discussion that took place.

From what I remember about the conclusion, there were still trade-offs to consider.