Skip to content

Commit

Permalink
git-pr-create: be explicit about CC:d mailing lists
Browse files Browse the repository at this point in the history 
Reviewed-by: rwestberg
  • Loading branch information
@edvbld
edvbld committed Aug 29, 2020
1 parent e3aa0f1 commit e954044
Show file tree
Hide file tree
Showing 8 changed files with 199 additions and 53 deletions.
2 changes: 1 addition & 1 deletion bots/pr/src/main/java/org/openjdk/skara/bots/pr/LabelerWorkItem.java
View file
Expand Up @@ -44,7 +44,7 @@ public String toString() {

private Set<String> getLabels(Repository localRepo) throws IOException {
var files = PullRequestUtils.changedFiles(pr, localRepo);
return bot.labelConfiguration().fromChanges(files);
return bot.labelConfiguration().label(files);
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotBuilder.java
View file
Expand Up @@ -23,6 +23,7 @@
package org.openjdk.skara.bots.pr;

import org.openjdk.skara.forge.HostedRepository;
import org.openjdk.skara.forge.LabelConfiguration;
import org.openjdk.skara.issuetracker.IssueProject;

import java.nio.file.Path;
Expand All @@ -33,7 +34,7 @@ public class PullRequestBotBuilder {
private HostedRepository repo;
private HostedRepository censusRepo;
private String censusRef = "master";
private LabelConfiguration labelConfiguration = LabelConfiguration.newBuilder().build();
private LabelConfiguration labelConfiguration = LabelConfiguration.builder().build();
private Map<String, String> externalCommands = Map.of();
private Map<String, String> blockingCheckLabels = Map.of();
private Set<String> readyLabels = Set.of();
Expand Down
28 changes: 3 additions & 25 deletions bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotFactory.java
View file
Expand Up @@ -23,6 +23,7 @@
package org.openjdk.skara.bots.pr;

import org.openjdk.skara.bot.*;
import org.openjdk.skara.forge.LabelConfiguration;
import org.openjdk.skara.json.*;

import java.util.*;
Expand Down Expand Up @@ -64,31 +65,8 @@ public List<Bot> create(BotConfiguration configuration) {

var labelConfigurations = new HashMap<String, LabelConfiguration>();
for (var labelGroup : specific.get("labels").fields()) {
var labelConfiguration = LabelConfiguration.newBuilder();
if (labelGroup.value().contains("matchers")) {
var matchers = labelGroup.value().get("matchers").fields().stream()
.collect(Collectors.toMap(JSONObject.Field::name,
field -> field.value().stream()
.map(JSONValue::asString)
.map(Pattern::compile)
.collect(Collectors.toList())));
matchers.forEach(labelConfiguration::addMatchers);
}
if (labelGroup.value().contains("groups")) {
var groups = labelGroup.value().get("groups").fields().stream()
.collect(Collectors.toMap(JSONObject.Field::name,
field -> field.value().stream()
.map(JSONValue::asString)
.collect(Collectors.toList())));
groups.forEach(labelConfiguration::addGroup);
}
if (labelGroup.value().contains("extra")) {
var extra = labelGroup.value().get("extra").stream()
.map(JSONValue::asString)
.collect(Collectors.toList());
extra.forEach(labelConfiguration::addExtra);
}
labelConfigurations.put(labelGroup.name(), labelConfiguration.build());
labelConfigurations.put(labelGroup.name(),
LabelConfiguration.fromJSON(labelGroup.value()));
}

for (var repo : specific.get("repositories").fields()) {
Expand Down
9 changes: 5 additions & 4 deletions bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelTests.java
View file
Expand Up @@ -22,6 +22,7 @@
*/
package org.openjdk.skara.bots.pr;

import org.openjdk.skara.forge.LabelConfiguration;
import org.junit.jupiter.api.*;
import org.openjdk.skara.test.*;

Expand All @@ -44,7 +45,7 @@ void simple(TestInfo testInfo) throws IOException {
var censusBuilder = credentials.getCensusBuilder()
.addReviewer(integrator.forge().currentUser().id())
.addCommitter(author.forge().currentUser().id());
var labelConfiguration = LabelConfiguration.newBuilder()
var labelConfiguration = LabelConfiguration.builder()
.addMatchers("1", List.of(Pattern.compile("cpp$")))
.addMatchers("2", List.of(Pattern.compile("hpp$")))
.addGroup("group", List.of("1", "2"))
Expand Down Expand Up @@ -134,7 +135,7 @@ void adjustAutoApplied(TestInfo testInfo) throws IOException {
var censusBuilder = credentials.getCensusBuilder()
.addReviewer(integrator.forge().currentUser().id())
.addCommitter(author.forge().currentUser().id());
var labelConfiguration = LabelConfiguration.newBuilder()
var labelConfiguration = LabelConfiguration.builder()
.addMatchers("1", List.of(Pattern.compile("cpp$")))
.addMatchers("2", List.of(Pattern.compile("hpp$")))
.addGroup("group", List.of("1", "2"))
Expand Down Expand Up @@ -202,7 +203,7 @@ void overrideAutoApplied(TestInfo testInfo) throws IOException {
var censusBuilder = credentials.getCensusBuilder()
.addReviewer(integrator.forge().currentUser().id())
.addCommitter(author.forge().currentUser().id());
var labelConfiguration = LabelConfiguration.newBuilder()
var labelConfiguration = LabelConfiguration.builder()
.addMatchers("1", List.of(Pattern.compile("cpp$")))
.addMatchers("2", List.of(Pattern.compile("hpp$")))
.addGroup("group", List.of("1", "2"))
Expand Down Expand Up @@ -264,7 +265,7 @@ void commandAuthor(TestInfo testInfo) throws IOException {
.addCommitter(committer.forge().currentUser().id())
.addAuthor(author.forge().currentUser().id())
.addAuthor(other.forge().currentUser().id());
var labelConfiguration = LabelConfiguration.newBuilder()
var labelConfiguration = LabelConfiguration.builder()
.addMatchers("1", List.of(Pattern.compile("cpp$")))
.addMatchers("2", List.of(Pattern.compile("hpp$")))
.addGroup("group", List.of("1", "2"))
Expand Down
3 changes: 2 additions & 1 deletion bots/pr/src/test/java/org/openjdk/skara/bots/pr/LabelerTests.java
View file
Expand Up @@ -23,6 +23,7 @@
package org.openjdk.skara.bots.pr;

import org.openjdk.skara.test.*;
import org.openjdk.skara.forge.LabelConfiguration;

import org.junit.jupiter.api.*;

Expand All @@ -41,7 +42,7 @@ void simple(TestInfo testInfo) throws IOException {
var author = credentials.getHostedRepository();
var reviewer = credentials.getHostedRepository();

var labelConfiguration = LabelConfiguration.newBuilder()
var labelConfiguration = LabelConfiguration.builder()
.addMatchers("test1", List.of(Pattern.compile("a.txt")))
.addMatchers("test2", List.of(Pattern.compile("b.txt")))
.build();
Expand Down
133 changes: 129 additions & 4 deletions cli/src/main/java/org/openjdk/skara/cli/pr/GitPrCreate.java
View file
Expand Up @@ -26,17 +26,18 @@
import org.openjdk.skara.cli.GitPublish;
import org.openjdk.skara.cli.GitJCheck;
import org.openjdk.skara.vcs.Branch;
import org.openjdk.skara.vcs.ReadOnlyRepository;
import org.openjdk.skara.vcs.openjdk.CommitMessageParsers;
import org.openjdk.skara.forge.Forge;
import org.openjdk.skara.forge.LabelConfiguration;
import org.openjdk.skara.json.JSON;

import static org.openjdk.skara.cli.pr.Utils.*;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.*;
import java.util.stream.Collectors;

public class GitPrCreate {
Expand All @@ -56,6 +57,11 @@ public class GitPrCreate {
.describe("NAME")
.helptext("Name of target branch, defaults to 'master'")
.optional(),
Option.shortcut("")
.fullname("cc")
.describe("MAILING LISTS")
.helptext("Mailing lists to CC for inital RFR e-mail")
.optional(),
Switch.shortcut("")
.fullname("ignore-workspace")
.helptext("Ignore local changes in worktree and staging area when creating pull request")
Expand Down Expand Up @@ -97,6 +103,33 @@ public class GitPrCreate {
.optional()
);


private static LabelConfiguration labelConfiguration(Forge forge, String project) throws IOException {
var group = project.split("/")[0];
var skaraRemoteRepo = forge.repository(group + "/skara").orElseThrow(() ->
new IOException("error: could not resolve Skara repository")
);
var rules = skaraRemoteRepo.fileContents("config/mailinglist/rules/jdk.json", "master");
var json = JSON.parse(rules);
return LabelConfiguration.fromJSON(json);
}

private static Set<String> suggestedLabels(ReadOnlyRepository repo, Forge forge, String project, String targetRef, String headRef) throws IOException {
var config = labelConfiguration(forge, project);
var baseHash = repo.resolve(targetRef).orElseThrow(() ->
new IOException("error: cannot resolve " + targetRef)
);
var headHash = repo.resolve(headRef).orElseThrow(() ->
new IOException("error: cannot resolve " + headRef)
);
var status = repo.status(baseHash, headHash);
var files = status.stream()
.filter(e -> !e.status().isDeleted())
.map(e -> e.target().path().get())
.collect(Collectors.toSet());
return config.label(files);
}

public static void main(String[] args) throws IOException, InterruptedException {
var parser = new ArgumentParser("git-pr", flags, inputs);
var arguments = parse(parser, args);
Expand Down Expand Up @@ -269,6 +302,84 @@ public static void main(String[] args) throws IOException, InterruptedException
}
}

var mailingLists = new ArrayList<String>();
var parentProject = projectName(parentRepo.url());
var isTargetingJDKRepo = parentProject.endsWith("jdk");
var cc = getOption("cc", "create", arguments);
var isCCManual = cc != null && !cc.equals("auto");
if (!isTargetingJDKRepo && isCCManual) {
System.out.println("error: you cannot manually CC additional mailing lists for " + parentProject);
System.exit(1);
}
if (isTargetingJDKRepo) {
if (isCCManual) {
var config = labelConfiguration(host, parentProject);
var lists = cc.split(",");
for (var input : lists) {
var label = input;
if (label.endsWith("@openjdk.java.net")) {
label = input.split("@")[0];
}
if (label.endsWith("-dev")) {
label = label.replace("-dev", "");
}
if (!config.isAllowed(label) && !config.isAllowed(label + "-dev")) {
System.out.println("error: the mailing list \"" + label +
"-dev@openjdk.java.net\" is not applicable, aborting.");
System.exit(1);
}
}
System.out.println("You have chosen the following mailing lists to be CC:d for the \"RFR\" e-mail:");
for (var input : lists) {
String list = null;
if (input.endsWith("@openjdk.java.net")) {
list = input;
} else if (input.endsWith("-dev")) {
list = input + "@openjdk.java.net";
} else {
list = input + "-dev@openjdk.java.net";
}
System.out.println("- " + list);
mailingLists.add(list);
}
} else {
var suggested = suggestedLabels(repo, host, parentProject, targetBranch, headRef);
System.out.println("The following mailing lists will be CC:d for the \"RFR\" e-mail:");
for (var label : suggested) {
String list = null;
if (label.endsWith("-dev")) {
list = label + "@openjdk.java.net";
} else {
list = label + "-dev@openjdk.java.net";
}
if (cc == null) {
System.out.println("- " + list);
}
mailingLists.add(list);
}
}
if (cc == null || !cc.equals("auto")) {
System.out.println("");
System.out.print("Do you want to proceed with this mailing list selection? [Y/n]: ");
var scanner = new Scanner(System.in);
var answer = scanner.nextLine().toLowerCase();
while (!(answer.equals("y") || answer.equals("n") || answer.isEmpty())) {
System.out.print("Please answer with 'y', 'n' or empty for the default choice: ");
answer = scanner.nextLine().toLowerCase();
}
if (!(answer.isEmpty() || answer.equals("y"))) {
System.out.println("");
System.out.println("error: user not satisfied with mailing list selection, aborting.");
if (cc == null) {
System.out.println(" To specify mailing lists manually, use the --cc option.");
} else if (cc.equals("auto")) {
System.out.println(" You have set --cc=auto, you can use --cc to specify mailing lists manually");
}
System.exit(1);
}
}
}

var project = jbsProjectFromJcheckConf(repo, targetBranch);
var issue = getIssue(currentBranch, project);
var file = Files.createTempFile("PULL_REQUEST_", ".md");
Expand Down Expand Up @@ -327,6 +438,13 @@ public static void main(String[] args) throws IOException, InterruptedException
}
appendPaddedHTMLComment(file, "Repository: " + parentRepo.webUrl());
appendPaddedHTMLComment(file, "Branch: " + targetBranch);
if (!mailingLists.isEmpty()) {
appendPaddedHTMLComment(file, "");
appendPaddedHTMLComment(file, "The following mailing lists will be CC:d for the \"RFR\" e-mail:");
for (var list : mailingLists) {
appendPaddedHTMLComment(file, "- " + list);
}
}

var success = spawnEditor(repo, file);
if (!success) {
Expand Down Expand Up @@ -355,6 +473,13 @@ public static void main(String[] args) throws IOException, InterruptedException
System.exit(1);
}

if (isCCManual && !mailingLists.isEmpty()) {
var arg = mailingLists.stream()
.map(l -> l.split("@")[0].replace("-dev", ""))
.collect(Collectors.joining(","));
body.add("/cc " + arg);
}

var isDraft = getSwitch("draft", "create", arguments);
if (upstream.isEmpty() && shouldPublish) {
GitPublish.main(new String[] { "--quiet", remote });
Expand Down

1 comment on commit e954044

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented on e954044 Aug 29, 2020

Choose a reason for hiding this comment

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

Review

Please sign in to comment.