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.

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

553: Add informational message when automatic labeling comes up empty #783

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -24,6 +24,7 @@

import org.openjdk.skara.bot.WorkItem;
import org.openjdk.skara.forge.*;
import org.openjdk.skara.issuetracker.Comment;
import org.openjdk.skara.vcs.Repository;

import java.io.*;
@@ -33,6 +34,8 @@
import java.util.stream.Collectors;

public class LabelerWorkItem extends PullRequestWorkItem {
private static final String initialLabelMessage = "<!-- PullRequestBot initial label help comment -->";

LabelerWorkItem(PullRequestBot bot, PullRequest pr, Consumer<RuntimeException> errorHandler) {
super(bot, pr, errorHandler);
}
@@ -47,6 +50,60 @@ public String toString() {
return bot.labelConfiguration().label(files);
}

private Optional<Comment> findComment(List<Comment> comments, String marker) {
var self = pr.repository().forge().currentUser();
return comments.stream()
.filter(comment -> comment.author().equals(self))
.filter(comment -> comment.body().contains(marker))
.findAny();
}

private void updateLabelMessage(List<Comment> comments, List<String> newLabels) {
var existing = findComment(comments, initialLabelMessage);
if (existing.isPresent()) {
// Only add the comment once per PR
return;
}

var message = new StringBuilder();
message.append("@");
message.append(pr.author().userName());
message.append(" ");

if (newLabels.isEmpty()) {
message.append("To determine the appropriate audience for reviewing this pull request, one or more ");
message.append("labels corresponding to different subsystems will normally be applied automatically. ");
message.append("However, no automatic labelling rule matches the changes in this pull request. ");
message.append("In order to have an RFR email automatically sent to the correct mailing list, you will ");
message.append("need to add one or more labels manually using the `/label add \"label\"` command. ");
message.append("The following labels are valid: ");
var labels = bot.labelConfiguration().allowed().stream()
.sorted()
.map(label -> "`" + label + "`")
.collect(Collectors.joining(", "));
message.append(labels);
message.append(".");
} else {
message.append("The following labels will be automatically applied to this pull request: ");
var labels = newLabels.stream()
.sorted()
.map(label -> "`" + label + "`")
.collect(Collectors.joining(", "));
message.append("`");
message.append(labels);
message.append("`. When this pull request is ready to be reviewed, an RFR email will be sent to the ");
message.append("corresponding mailing list");
if (newLabels.size() > 1) {
message.append("s");
}
message.append(". If you would like to change these labels, use the `/label (add|remove) \"label\"` command.");
}

message.append("\n");
message.append(initialLabelMessage);
pr.addComment(message.toString());
}

@Override
public Collection<WorkItem> run(Path scratchPath) {
if (bot.currentLabels().containsKey(pr.headHash())) {
@@ -73,10 +130,12 @@ public String toString() {
}

// Add all labels not already set that are not manually removed
newLabels.stream()
var labelsToAdd = newLabels.stream()
.filter(label -> !currentLabels.contains(label))
.filter(label -> !manuallyRemoved.contains(label))
.forEach(pr::addLabel);
.collect(Collectors.toList());
updateLabelMessage(comments, labelsToAdd);
labelsToAdd.forEach(pr::addLabel);

// Remove set labels no longer present unless it has been manually added
currentLabels.stream()
@@ -162,6 +162,8 @@ void adjustAutoApplied(TestInfo testInfo) throws IOException {
// The bot should have applied one label automatically
TestBotRunner.runPeriodicItems(prBot);
assertEquals(Set.of("2", "rfr"), new HashSet<>(pr.labels()));
assertLastCommentContains(pr, "The following labels will be automatically applied");
assertLastCommentContains(pr, "`2`");

// It will refuse to remove it
pr.addComment("/label remove 2");
@@ -231,6 +233,8 @@ void overrideAutoApplied(TestInfo testInfo) throws IOException {
// The bot will not add any label automatically
TestBotRunner.runPeriodicItems(prBot);
assertEquals(Set.of("1", "rfr"), new HashSet<>(pr.labels()));
assertEquals(1, pr.comments().size());
assertLastCommentContains(pr, "The `1` label was successfully added.");

// Add another file to trigger a group match
Files.writeString(localRepoFolder.resolve("test.cpp"), "Hello there");
@@ -33,6 +33,7 @@
import java.util.regex.Pattern;

import static org.junit.jupiter.api.Assertions.*;
import static org.openjdk.skara.bots.pr.PullRequestAsserts.assertLastCommentContains;

class LabelerTests {
@Test
@@ -69,6 +70,8 @@ void simple(TestInfo testInfo) throws IOException {
// Check the status - only the rfr label should be set
TestBotRunner.runPeriodicItems(labelBot);
assertEquals(Set.of("rfr"), new HashSet<>(pr.labels()));
assertLastCommentContains(pr, "However, no automatic labelling rule matches the changes in this pull request.");
assertLastCommentContains(pr, "The following labels are valid: `test1`, `test2`");

var fileA = localRepoFolder.resolve("a.txt");
Files.writeString(fileA, "Hello");
ProTip! Use n and p to navigate between commits in a pull request.