RFR: 304: Squash additional commits into merge commits for merge PRs
Erik Helin
ehelin at openjdk.java.net
Thu Mar 19 08:41:29 UTC 2020
On Thu, 19 Mar 2020 07:39:11 GMT, Robin Westberg <rwestberg 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
Looks good in general, just a few inline comments!
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?
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"?
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?
-------------
PR: https://git.openjdk.java.net/skara/pull/519
More information about the skara-dev
mailing list