Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

214: Improve the message given when a change can be integrated #343

Closed
wants to merge 2 commits into from
Closed

214: Improve the message given when a change can be integrated #343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
91d414c
Adjust the message
Dec 19, 2019
ebda6a9
Adapt tests to updated message
Dec 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 14 additions & 12 deletions bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java
View file
Open in desktop
Expand Up @@ -358,13 +358,20 @@ private String getMergeReadyComment(String commitMessage, List<Review> reviews,
var message = new StringBuilder();
message.append("@");
message.append(pr.author().userName());
message.append(" This change can now be integrated. The commit message will be:\n");
message.append(" This change now passes all automated pre-integration checks. When the change also ");
message.append("fulfills all [project specific requirements](https://github.com/");
message.append(pr.repository().name());
message.append("/blob/");
message.append(pr.targetRef());
message.append("CONTRIBUTING.md), type `/integrate` in a new comment to proceed. After integration, ");
message.append("the commit message will be:\n");
message.append("```\n");
message.append(commitMessage);
message.append("\n```\n");

message.append("- If you would like to add a summary, use the `/summary` command.\n");
message.append("- To list additional contributors, use the `/contributor` command.\n");
message.append("- To credit additional contributors, use the `/contributor` command.\n");
message.append("- To add additional solved issues, use the `/solves` command.\n");

var divergingCommits = prInstance.divergingCommits();
if (divergingCommits.size() > 0) {
Expand All @@ -379,15 +386,10 @@ private String getMergeReadyComment(String commitMessage, List<Review> reviews,
}
message.append("pushed to the `");
message.append(pr.targetRef());
message.append("` branch:\n");
var commitList = divergingCommits.stream()
.map(commit -> " * " + commit.hash().hex() + ": " + commit.message().get(0))
.collect(Collectors.joining("\n"));
message.append(commitList);
message.append("\n\n");
message.append("` branch. ");
if (rebasePossible) {
message.append("Since there are no conflicts, your changes will automatically be rebased on top of the ");
message.append("above commits when integrating. If you prefer to do this manually, please merge `");
message.append("Since there are no conflicts, your changes will automatically be rebased on top of ");
message.append("these commits when integrating. If you prefer to do this manually, please merge `");
message.append(pr.targetRef());
message.append("` into your branch first.\n");
} else {
Expand Down Expand Up @@ -420,15 +422,15 @@ private String getMergeReadyComment(String commitMessage, List<Review> reviews,
}
if (rebasePossible) {
message.append("\n\n");
message.append("- To flag this PR as ready for integration with the above commit message, type ");
message.append("➡️ To flag this PR as ready for integration with the above commit message, type ");
message.append("`/integrate` in a new comment. (Afterwards, your sponsor types ");
message.append("`/sponsor` in a new comment to perform the integration).\n");
}
} else if (rebasePossible) {
if (divergingCommits.size() > 0) {
message.append("\n");
}
message.append("- To integrate this PR with the above commit message, type ");
message.append("➡️ To integrate this PR with the above commit message, type ");
message.append("`/integrate` in a new comment.\n");
}
message.append(mergeReadyMarker);
Expand Down
2 changes: 1 addition & 1 deletion bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java
View file
Open in desktop
Expand Up @@ -598,7 +598,7 @@ void cannotRebase(TestInfo testInfo) throws IOException {

// The bot should no longer detect a conflict
updated = pr.comments().stream()
.filter(comment -> comment.body().contains("change can now be integrated"))
.filter(comment -> comment.body().contains("change now passes all automated"))
.count();
assertEquals(1, updated);

Expand Down
2 changes: 1 addition & 1 deletion bots/pr/src/test/java/org/openjdk/skara/bots/pr/ContributorTests.java
View file
Open in desktop
Expand Up @@ -106,7 +106,7 @@ void simple(TestInfo testInfo) throws IOException {
assertEquals("Co-authored-by: Test Person <test@test.test>", creditLine);

var pushed = pr.comments().stream()
.filter(comment -> comment.body().contains("change can now be integrated"))
.filter(comment -> comment.body().contains("change now passes all automated"))
.count();
assertEquals(1, pushed);

Expand Down
6 changes: 3 additions & 3 deletions bots/pr/src/test/java/org/openjdk/skara/bots/pr/IntegrateTests.java
View file
Open in desktop
Expand Up @@ -343,7 +343,7 @@ void mergeNotification(TestInfo testInfo) throws IOException {

// The bot should reply with an instructional message (and only one)
var pushed = pr.comments().stream()
.filter(comment -> comment.body().contains("change can now be integrated"))
.filter(comment -> comment.body().contains("change now passes all automated"))
.filter(comment -> comment.body().contains("Reviewed-by: integrationreviewer3"))
.count();
assertEquals(1, pushed);
Expand All @@ -370,7 +370,7 @@ void mergeNotification(TestInfo testInfo) throws IOException {

// The instructional message should have been updated
pushed = pr.comments().stream()
.filter(comment -> comment.body().contains("change can now be integrated"))
.filter(comment -> comment.body().contains("change now passes all automated"))
.filter(comment -> comment.body().contains("Reviewed-by: integrationreviewer3"))
.count();
assertEquals(1, pushed);
Expand All @@ -382,7 +382,7 @@ void mergeNotification(TestInfo testInfo) throws IOException {

// The instructional message should have been updated
pushed = pr.comments().stream()
.filter(comment -> comment.body().contains("change can now be integrated"))
.filter(comment -> comment.body().contains("change now passes all automated"))
.filter(comment -> comment.body().contains("Reviewed-by: integrationreviewer3, integrationreviewer2"))
.count();
assertEquals(1, pushed);
Expand Down
5 changes: 3 additions & 2 deletions bots/pr/src/test/java/org/openjdk/skara/bots/pr/SolvesTests.java
View file
Open in desktop
Expand Up @@ -22,12 +22,13 @@
*/
package org.openjdk.skara.bots.pr;

import org.junit.jupiter.api.*;
import org.openjdk.skara.forge.Review;
import org.openjdk.skara.issuetracker.Comment;
import org.openjdk.skara.test.*;
import org.openjdk.skara.vcs.Repository;

import org.junit.jupiter.api.*;

import java.io.IOException;
import java.util.*;

Expand Down Expand Up @@ -117,7 +118,7 @@ void simple(TestInfo testInfo) throws IOException {

// The commit message preview should contain the additional issues
var preview = pr.comments().stream()
.filter(comment -> comment.body().contains("The commit message will be"))
.filter(comment -> comment.body().contains("the commit message will be"))
.map(Comment::body)
.findFirst()
.orElseThrow();
Expand Down