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

Jump to bottom

git-pr-create: be explicit about CC:d mailing lists #750

Closed
wants to merge 3 commits into from
Closed

git-pr-create: be explicit about CC:d mailing lists #750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b222ed
git-pr: add --cc flag to git-pr-create
edvbld Aug 27, 2020
f88fd37
git-pr-create: be explicit about CC:d mailing lists
edvbld Aug 28, 2020
59e2cfb
Fix tests
edvbld Aug 28, 2020
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
2 changes: 1 addition & 1 deletion bots/pr/src/main/java/org/openjdk/skara/bots/pr/LabelerWorkItem.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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