Still, TFA raises a good point, which is that the diff should be between the feature branch and current master, not master at the time the branch was created.
I disagree. It should be both. Specifically because what was tested and run by the person sending in the request was the diff to the master at the time the branch was created.
Please let that sink in. Nobody ever tested the diff that this is going to be showing to the user.
But once you click the "Merge" button in GitHub, nobody has ever seen or tested the new state of master.
I'd rather see the diff that's going to go into master (tested or not) than see a diff that effectively means nothing (as it will never be applied to anything).
That is not necessarily true. The test results (e.g. travis, but you can use for instance Jenkins for your Github repository) shown next to the merge button are the result of a merge between the pull request branch and the branch you want to merge into. That's why when you do an ls-remote, you will see something like:
one is the head of the pull request, the other is the result of the merge that you will end up with. Though I'm not sure Github will trigger another build when the target branch moves.
That's how we build using Jenkins - it's a nice feature, but it doesn't retrigger when the target branch changes. So, while it's useful so far as that it's better than simply building the branch, it doesn't ensure that "what you tested is what you get."
In the Rust project, the merging itself is automated and the merge bot (bors) runs the test after having effected the merge. The merging is attempted after the branch has passed review.
That doesn't preclude running e.g. travis to get the branch's own status before the review happens.
If you use the Travis CI integration, it actually tests the result of merging a PR, rather than the branch being PRed. This seems to solve a lot of problems on projects with thorough testing.
I disagree still. Fundamentally, I agree that it is important. I'd go further and say that these two diffs should be similar. If they are dissimilar, then there should be a red flag.
That's a good point. The diffs would be more useful both the feature branch's changes (Bob's changes in the article's example) and master's diverged changes were highlighted (in different colors):
