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

Add an issue link to RFR mails if the PR title contains one #149

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

Always

Just for now

@@ -59,12 +59,14 @@ static String composeConversation(PullRequestInstance prInstance, URI webrev) {
if (filteredBody.isEmpty()) {
filteredBody = prInstance.pr().getTitle().strip();
}
var issueString = prInstance.issueUrl().map(url -> " Issue: " + url + "\n").orElse("");
return filteredBody + "\n\n" +
infoSeparator + "\n\n" +
"Commits:\n" +
commitMessages + "\n\n" +
"Changes: " + prInstance.changeUrl() + "\n" +
" Webrev: " + webrev.toString() + "\n" +
issueString +
" Stats: " + prInstance.stats(prInstance.baseHash(), prInstance.headHash()) + "\n" +
" Patch: " + prInstance.diffUrl() + "\n" +
" Fetch: " + prInstance.fetchCommand() + "\n\n" +
@@ -73,19 +75,22 @@ static String composeConversation(PullRequestInstance prInstance, URI webrev) {

static String composeRebaseComment(PullRequestInstance prInstance, URI fullWebrev) {
var commitMessages = prInstance.formatCommitMessages(prInstance.baseHash(), prInstance.headHash(), ArchiveMessages::formatCommit);
var issueString = prInstance.issueUrl().map(url -> " Issue: " + url + "\n").orElse("");
return "The pull request has been updated with a complete new set of changes (possibly due to a rebase).\n\n" +
infoSeparator + "\n\n" +
"Commits:\n" +
commitMessages + "\n\n" +
"Changes: " + prInstance.changeUrl() + "\n" +
" Webrev: " + fullWebrev.toString() + "\n" +
issueString +
" Stats: " + prInstance.stats(prInstance.baseHash(), prInstance.headHash()) + "\n" +
" Patch: " + prInstance.diffUrl() + "\n" +
" Fetch: " + prInstance.fetchCommand() + "\n\n" +
replyFooter(prInstance); }

static String composeIncrementalComment(Hash lastHead, PullRequestInstance prInstance, URI fullWebrev, URI incrementalWebrev) {
var newCommitMessages = prInstance.formatCommitMessages(lastHead, prInstance.headHash(), ArchiveMessages::formatCommit);
var issueString = prInstance.issueUrl().map(url -> " Issue: " + url + "\n").orElse("");
return "The pull request has been updated with additional changes.\n\n" +
infoSeparator + "\n\n" +
"Added commits:\n" +
@@ -96,6 +101,7 @@ static String composeIncrementalComment(Hash lastHead, PullRequestInstance prIns
"Webrevs:\n" +
" - full: " + fullWebrev.toString() + "\n" +
" - incr: " + incrementalWebrev.toString() + "\n\n" +
issueString +
" Stats: " + prInstance.stats(lastHead, prInstance.headHash()) + "\n" +
" Patch: " + prInstance.diffUrl() + "\n" +
" Fetch: " + prInstance.fetchCommand() + "\n\n" +
@@ -227,7 +227,12 @@ public void run(Path scratchPath) {
}

var census = CensusInstance.create(bot.censusRepo(), bot.censusRef(), scratchPath.resolve("census"), pr);
var prInstance = new PullRequestInstance(scratchPath.resolve("mlbridge-mergebase"), pr);
var jbs = census.configuration().general().jbs();
if (jbs == null) {
jbs = census.configuration().general().project();
}
var prInstance = new PullRequestInstance(scratchPath.resolve("mlbridge-mergebase"), pr, bot.issueTracker(),
jbs.toUpperCase());
var reviewArchive = new ReviewArchive(bot.emailAddress(), prInstance, census, sentMails);
var webrevPath = scratchPath.resolve("mlbridge-webrevs");
var listServer = MailingListServerFactory.createMailmanServer(bot.listArchive(), bot.smtpServer());
@@ -45,14 +45,15 @@
private final WebrevStorage webrevStorage;
private final Set<String> readyLabels;
private final Map<String, Pattern> readyComments;
private final URI issueTracker;
private final PullRequestUpdateCache updateCache;

MailingListBridgeBot(EmailAddress from, HostedRepository repo, HostedRepository archive,
HostedRepository censusRepo, String censusRef, EmailAddress list,
Set<String> ignoredUsers, Set<Pattern> ignoredComments, URI listArchive, String smtpServer,
HostedRepository webrevStorageRepository, String webrevStorageRef,
Path webrevStorageBase, URI webrevStorageBaseUri, Set<String> readyLabels,
Map<String, Pattern> readyComments) {
Map<String, Pattern> readyComments, URI issueTracker) {
emailAddress = from;
codeRepo = repo;
archiveRepo = archive;
@@ -65,6 +66,7 @@
this.smtpServer = smtpServer;
this.readyLabels = readyLabels;
this.readyComments = readyComments;
this.issueTracker = issueTracker;

this.webrevStorage = new WebrevStorage(webrevStorageRepository, webrevStorageRef, webrevStorageBase,
webrevStorageBaseUri, from);
@@ -123,6 +125,10 @@ WebrevStorage webrevStorage() {
return readyComments;
}

URI issueTracker() {
return issueTracker;
}

@Override
public List<WorkItem> getPeriodicItems() {
List<WorkItem> ret = new LinkedList<>();
@@ -61,6 +61,7 @@ public String name() {
var webrevWeb = specific.get("webrevs").get("web").asString();

var archiveRepo = configuration.repository(specific.get("archive").asString());
var issueTracker = URIBuilder.base(specific.get("issues").asString()).build();

var allListNames = new HashSet<EmailAddress>();
var allRepositories = new HashSet<HostedRepository>();
@@ -84,7 +85,8 @@ public String name() {
censusRepo, censusRef,
list, ignoredUsers, ignoredComments, listArchive, listSmtp,
webrevRepo, webrevRef, Path.of(folder),
URIBuilder.base(webrevWeb).build(), readyLabels, readyComments);
URIBuilder.base(webrevWeb).build(), readyLabels, readyComments,
issueTracker);
ret.add(bot);

allListNames.add(list);
@@ -23,10 +23,14 @@
package org.openjdk.skara.bots.mlbridge;

import org.openjdk.skara.host.PullRequest;
import org.openjdk.skara.host.network.URIBuilder;
import org.openjdk.skara.vcs.*;
import org.openjdk.skara.vcs.openjdk.Issue;

import java.io.*;
import java.net.URI;
import java.nio.file.Path;
import java.util.Optional;
import java.util.stream.Collectors;

class PullRequestInstance {
@@ -35,9 +39,13 @@
private final Hash targetHash;
private final Hash headHash;
private final Hash baseHash;
private final URI issueTracker;
private final String projectPrefix;

PullRequestInstance(Path localRepoPath, PullRequest pr) {
PullRequestInstance(Path localRepoPath, PullRequest pr, URI issueTracker, String projectPrefix) {
this.pr = pr;
this.issueTracker = issueTracker;
this.projectPrefix = projectPrefix;

// Materialize the PR's target ref
try {
@@ -101,6 +109,11 @@ String stats(Hash base, Hash head) {
}
}

Optional<String> issueUrl() {
var issue = Issue.fromString(pr.getTitle());
return issue.map(value -> URIBuilder.base(issueTracker).appendPath(projectPrefix + "-" + value.id()).build().toString());
}

@FunctionalInterface
interface CommitFormatter {
String format(Commit commit);
@@ -71,7 +71,8 @@ void simpleArchive(TestInfo testInfo) throws IOException {
listServer.getArchive(), listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// The mailing list as well
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP());
@@ -134,7 +135,8 @@ void rememberBridged(TestInfo testInfo) throws IOException {
listServer.getArchive(), listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// The mailing list as well
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP());
@@ -111,7 +111,8 @@ void simpleArchive(TestInfo testInfo) throws IOException {
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of("rfr"), Map.of(ignored.host().getCurrentUserDetails().userName(),
Pattern.compile("ready")));
Pattern.compile("ready")),
URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var localRepo = CheckableRepository.init(tempFolder.path(), author.getRepositoryType());
@@ -123,7 +124,7 @@ void simpleArchive(TestInfo testInfo) throws IOException {
var editHash = CheckableRepository.appendAndCommit(localRepo, "A simple change",
"Change msg\n\nWith several lines");
localRepo.push(editHash, author.getUrl(), "edit", true);
var pr = credentials.createPullRequest(archive, "master", "edit", "This is a pull request");
var pr = credentials.createPullRequest(archive, "master", "edit", "1234: This is a pull request");
pr.setBody("This should not be ready");

// Run an archive pass
@@ -170,6 +171,8 @@ void simpleArchive(TestInfo testInfo) throws IOException {
assertTrue(archiveContains(archiveFolder.path(), "Webrev:"));
assertTrue(archiveContains(archiveFolder.path(), "http://www.test.test/"));
assertTrue(archiveContains(archiveFolder.path(), "webrev.00"));
assertTrue(archiveContains(archiveFolder.path(), "Issue:"));
assertTrue(archiveContains(archiveFolder.path(), "http://issues.test/browse/TSTPRJ-1234"));
assertTrue(archiveContains(archiveFolder.path(), "Fetch:"));
assertTrue(archiveContains(archiveFolder.path(), "^ - " + editHash.abbreviate() + ": Change msg"));
assertFalse(archiveContains(archiveFolder.path(), "With several lines"));
@@ -181,7 +184,7 @@ void simpleArchive(TestInfo testInfo) throws IOException {
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
var mail = conversations.get(0).first();
assertEquals("RFR: This is a pull request", mail.subject());
assertEquals("RFR: 1234: This is a pull request", mail.subject());
assertEquals(pr.getAuthor().fullName(), mail.author().fullName().orElseThrow());
assertEquals(noreplyAddress(archive), mail.author().address());
assertEquals(from, mail.sender());
@@ -260,7 +263,8 @@ void reviewComment(TestInfo testInfo) throws IOException {
listServer.getArchive(), listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
@@ -346,7 +350,8 @@ void combineComments(TestInfo testInfo) throws IOException {
listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
@@ -418,7 +423,8 @@ void commentThreading(TestInfo testInfo) throws IOException {
listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
@@ -526,7 +532,8 @@ void reviewContext(TestInfo testInfo) throws IOException {
listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
@@ -575,7 +582,8 @@ void multipleReviewContexts(TestInfo testInfo) throws IOException {
listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
@@ -642,7 +650,8 @@ void filterComments(TestInfo testInfo) throws IOException {
listServer.getArchive(), listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
@@ -699,7 +708,8 @@ void incrementalChanges(TestInfo testInfo) throws IOException {
listServer.getArchive(), listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
@@ -818,7 +828,8 @@ void rebased(TestInfo testInfo) throws IOException {
listServer.getArchive(), listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
@@ -909,7 +920,8 @@ void skipAddingExistingWebrev(TestInfo testInfo) throws IOException {
listServer.getArchive(), listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var localRepo = CheckableRepository.init(tempFolder.path(), author.getRepositoryType());
@@ -980,7 +992,8 @@ void notifyReviewVerdicts(TestInfo testInfo) throws IOException {
listServer.getArchive(), listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
@@ -1062,7 +1075,8 @@ void ignoreComments(TestInfo testInfo) throws IOException {
listServer.getArchive(), listServer.getSMTP(),
archive, "webrev", Path.of("test"),
URIBuilder.base("http://www.test.test/").build(),
Set.of(), Map.of());
Set.of(), Map.of(),
URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.