RFR: 304: Squash additional commits into merge commits for merge PRs

[Robin Westberg]

On Thu, 19 Mar 2020 08:34:49 GMT, Erik Helin <ehelin at openjdk.org> wrote:

>> Hi all,
>> 
>> Please review this change that squashes additional commits made on top of the first merge commit, to allow updating the
>> PR.
>> Best regards,
>> Robin
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 174:
> 
>> 173:                 }
>> 174:                 if (mergeCommitIndex >= commits.size() - 1) {
>> 175:                     ret.add("A Merge PR must contain a merge commit.");
> 
> What if you do a "Merge PR" with a single trivial merge just to merge to branches (that have identical commits). Too
> narrow of a use case to think about?

Not sure if that is something we would need to support, not even sure how to create such a merge commit. :)

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 175:
> 
>> 174:                 if (mergeCommitIndex >= commits.size() - 1) {
>> 175:                     ret.add("A Merge PR must contain a merge commit.");
>> 176:                 }
> 
> Related to my previous comment, should the message be "A Merge PR must contain at least one merge commit and one
> non-merge commit"?

Sure, can update that.

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestInstance.java line 153:
> 
>> 152:
>> 153:         localRepo.checkout(commits.get(mergeCommitIndex).hash(), true);
>> 154:         localRepo.squash(headHash);
> 
> Check that `mergeCommitIndex < commits.size()` before and otherwise throw an exception?

That should never happen (tm), the actual merge depends on jcheck passing first so we can assume that the PR is
well-formed.

-------------

PR: https://git.openjdk.java.net/skara/pull/519