Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
941: Warn about required merge after dependent PR integration
Reviewed-by: erikj
  • Loading branch information
rwestberg committed Apr 23, 2021
1 parent 1b3f3f0 commit f084dc2
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 3 deletions.
Expand Up @@ -76,8 +76,26 @@ public void onNewPullRequest(PullRequest pr, Path scratchPath) {
@Override
public void onStateChange(PullRequest pr, Path scratchPath, Issue.State oldState) {
if (pr.state() == Issue.State.CLOSED) {
PreIntegrations.retargetDependencies(pr);
var retargetedDependencies = PreIntegrations.retargetDependencies(pr);
deleteBranch(pr);
for (var retargeted : retargetedDependencies) {
retargeted.addComment("""
The dependent pull request has now been integrated, and the target branch of this pull request \
has been updated. This means that changes from the dependent pull request can start to show up \
as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, \
simply merge the latest changes from the new target branch into this pull request by running commands \
similar to these in the local repository for your personal fork:
```bash
git checkout %s
git fetch %s %s
git merge FETCH_HEAD
# if there are conflicts, follow the instructions given by git merge
git commit -m "Merge %s"
git push
```
""".formatted(pr.sourceRef(), pr.repository().webUrl(), pr.targetRef(), pr.targetRef()));
}
} else {
pushBranch(pr);
}
Expand Down
Expand Up @@ -232,6 +232,12 @@ void retarget(TestInfo testInfo) throws IOException {
// The follow-up PR should have been retargeted
followUpPr = repo.pullRequest(followUpPr.id());
assertEquals("master", followUpPr.targetRef());

// Instructions on how to adapt to the newly integrated changes should have been posted
var lastComment = followUpPr.comments().get(followUpPr.comments().size() - 1);
assertTrue(lastComment.body().contains("The dependent pull request has now"), lastComment.body());
assertTrue(lastComment.body().contains("git checkout source"), lastComment.body());
assertTrue(lastComment.body().contains("git commit -m \"Merge master\""), lastComment.body());
}
}
}
Expand Up @@ -22,7 +22,7 @@
*/
package org.openjdk.skara.forge;

import java.util.Optional;
import java.util.*;

public class PreIntegrations {
public static Optional<String> dependentPullRequestId(PullRequest pr) {
Expand All @@ -42,15 +42,18 @@ public static String preIntegrateBranch(PullRequest pr) {
return "pr/" + pr.id();
}

public static void retargetDependencies(PullRequest pr) {
public static Collection<PullRequest> retargetDependencies(PullRequest pr) {
var ret = new ArrayList<PullRequest>();
var dependentRef = preIntegrateBranch(pr);

var candidates = pr.repository().pullRequests();
for (var candidate : candidates) {
if (candidate.targetRef().equals(dependentRef)) {
candidate.setTargetRef(pr.targetRef());
ret.add(candidate);
}
}
return ret;
}

public static boolean isPreintegrationBranch(String name) {
Expand Down

1 comment on commit f084dc2

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.