Skip to content

Commit

Permalink
410: Update mailing list bridge subject decorations
Browse files Browse the repository at this point in the history 
Reviewed-by: ehelin
  • Loading branch information
@rwestberg
rwestberg committed Jun 15, 2020
1 parent f89126d commit ed7df80
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 20 deletions.
10 changes: 5 additions & 5 deletions bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java
View file
Expand Up @@ -156,7 +156,7 @@ static ArchiveItem from(PullRequest pr, Repository localRepo, HostUserToEmailAut
ZonedDateTime created, ZonedDateTime updated, Hash lastBase, Hash lastHead, Hash base,
Hash head, int index, ArchiveItem parent, String subjectPrefix, String threadPrefix) {
return new ArchiveItem(parent,"ha" + head.hex(), created, updated, pr.author(), Map.of("PR-Head-Hash", head.hex(), "PR-Base-Hash", base.hex()),
() -> String.format("Re: %s[Rev %02d] %s%s", subjectPrefix, index, threadPrefix + (threadPrefix.isEmpty() ? "" : ": "), pr.title()),
() -> String.format("Re: %s%s%s [v%d]", subjectPrefix, threadPrefix + (threadPrefix.isEmpty() ? "" : ": "), pr.title(), index + 1),
() -> "",
() -> {
if (lastBase.equals(base)) {
Expand Down Expand Up @@ -214,17 +214,17 @@ static ArchiveItem from(PullRequest pr, ReviewComment reviewComment, HostUserToE
() -> ArchiveMessages.composeReplyFooter(pr));
}

static ArchiveItem closedNotice(PullRequest pr, HostUserToEmailAuthor hostUserToEmailAuthor, ArchiveItem parent, String subjectPrefix, String threadPrefix) {
static ArchiveItem closedNotice(PullRequest pr, HostUserToEmailAuthor hostUserToEmailAuthor, ArchiveItem parent, String subjectPrefix) {
return new ArchiveItem(parent, "cn", pr.updatedAt(), pr.updatedAt(), pr.author(), Map.of("PR-Closed-Notice", "0"),
() -> String.format("Re: [Closed] %s%s%s", subjectPrefix, threadPrefix + (threadPrefix.isEmpty() ? "" : ": "), pr.title()),
() -> String.format("%sWithdrawn: %s", subjectPrefix, pr.title()),
() -> ArchiveMessages.composeReplyHeader(parent.createdAt(), hostUserToEmailAuthor.author(parent.author())),
() -> ArchiveMessages.composeClosedNotice(pr),
() -> ArchiveMessages.composeReplyFooter(pr));
}

static ArchiveItem integratedNotice(PullRequest pr, Repository localRepo, Commit commit, HostUserToEmailAuthor hostUserToEmailAuthor, ArchiveItem parent, String subjectPrefix, String threadPrefix) {
static ArchiveItem integratedNotice(PullRequest pr, Repository localRepo, Commit commit, HostUserToEmailAuthor hostUserToEmailAuthor, ArchiveItem parent, String subjectPrefix) {
return new ArchiveItem(parent, "in", pr.updatedAt(), pr.updatedAt(), pr.author(), Map.of("PR-Integrated-Notice", "0"),
() -> String.format("Re: [Integrated] %s%s%s", subjectPrefix, threadPrefix + (threadPrefix.isEmpty() ? "" : ": "), pr.title()),
() -> String.format("%sIntegrated: %s", subjectPrefix, pr.title()),
() -> ArchiveMessages.composeReplyHeader(parent.createdAt(), hostUserToEmailAuthor.author(parent.author())),
() -> ArchiveMessages.composeIntegratedNotice(pr, localRepo, commit),
() -> ArchiveMessages.composeReplyFooter(pr));
Expand Down
8 changes: 4 additions & 4 deletions bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java
View file
Expand Up @@ -89,7 +89,7 @@ private List<ArchiveItem> generateArchiveItems(List<Email> sentEmails, Repositor
}
} else {
if (pr.state() != Issue.State.OPEN) {
threadPrefix = "FYI";
threadPrefix = "Integrated";
}
}

Expand Down Expand Up @@ -152,18 +152,18 @@ private List<ArchiveItem> generateArchiveItems(List<Email> sentEmails, Repositor
if (hash.isPresent()) {
var commit = localRepo.lookup(hash.get());
if (!hasLegacyIntegrationNotice(localRepo, commit.orElseThrow())) {
var reply = ArchiveItem.integratedNotice(pr, localRepo, commit.orElseThrow(), hostUserToEmailAuthor, parent, subjectPrefix, threadPrefix);
var reply = ArchiveItem.integratedNotice(pr, localRepo, commit.orElseThrow(), hostUserToEmailAuthor, parent, subjectPrefix);
generated.add(reply);
}
} else {
throw new RuntimeException("PR " + pr.webUrl() + " has integrated label but no integration comment");
}
} else if (localRepo.isAncestor(pr.headHash(), pr.targetHash())) {
var commit = localRepo.lookup(pr.headHash());
var reply = ArchiveItem.integratedNotice(pr, localRepo, commit.orElseThrow(), hostUserToEmailAuthor, parent, subjectPrefix, threadPrefix);
var reply = ArchiveItem.integratedNotice(pr, localRepo, commit.orElseThrow(), hostUserToEmailAuthor, parent, subjectPrefix);
generated.add(reply);
} else if (threadPrefix.equals("RFR")) {
var reply = ArchiveItem.closedNotice(pr, hostUserToEmailAuthor, parent, subjectPrefix, threadPrefix);
var reply = ArchiveItem.closedNotice(pr, hostUserToEmailAuthor, parent, subjectPrefix);
generated.add(reply);
}
}
Expand Down
23 changes: 12 additions & 11 deletions bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java
View file
Expand Up @@ -335,7 +335,7 @@ void archiveIntegrated(TestInfo testInfo) throws IOException {

// The archive should now contain another entry
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertTrue(archiveContains(archiveFolder.path(), "Subject: Re: \\[Integrated\\] RFR: 1234: This is a pull request"));
assertTrue(archiveContains(archiveFolder.path(), "Subject: Integrated: 1234: This is a pull request"));
assertFalse(archiveContains(archiveFolder.path(), "\\[Closed\\]"));
}
}
Expand Down Expand Up @@ -478,7 +478,7 @@ void archiveDirectToIntegrated(TestInfo testInfo) throws IOException {

// The archive should now contain an entry
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertTrue(archiveContains(archiveFolder.path(), "Subject: FYI: 1234: This is a pull request"));
assertTrue(archiveContains(archiveFolder.path(), "Subject: Integrated: 1234: This is a pull request"));

var updatedHash = CheckableRepository.appendAndCommit(localRepo, "Another change");
localRepo.push(updatedHash, author.url(), "edit");
Expand All @@ -488,8 +488,8 @@ void archiveDirectToIntegrated(TestInfo testInfo) throws IOException {

// The archive should now contain another entry
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertTrue(archiveContains(archiveFolder.path(), "Subject: Re: \\[Rev 01\\] FYI: 1234: This is a pull request"));
assertFalse(archiveContains(archiveFolder.path(), "\\[Closed\\]"));
assertTrue(archiveContains(archiveFolder.path(), "Subject: Re: Integrated: 1234: This is a pull request \\[v2\\]"));
assertFalse(archiveContains(archiveFolder.path(), "Withdrawn"));
}
}

Expand Down Expand Up @@ -566,7 +566,7 @@ void archiveIntegratedRetainPrefix(TestInfo testInfo) throws IOException {

// The archive should now contain another entry - should retain the RFR thread prefix
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertTrue(archiveContains(archiveFolder.path(), "Subject: Re: \\[Rev 01\\] RFR: 1234: This is a pull request"));
assertTrue(archiveContains(archiveFolder.path(), "Subject: Re: RFR: 1234: This is a pull request \\[v2\\]"));
}
}

Expand Down Expand Up @@ -641,14 +641,15 @@ void archiveClosed(TestInfo testInfo) throws IOException {

// The archive should now contain another entry - should say that it is closed
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertEquals(1, archiveContainsCount(archiveFolder.path(), "Subject: Re: \\[Closed\\] RFR: 1234: This is a pull request"));
assertEquals(1, archiveContainsCount(archiveFolder.path(), "Subject: Withdrawn: 1234: This is a pull request"));

pr.addComment("Fair enough");

// Run another archive pass - only a single close notice should have been posted
TestBotRunner.runPeriodicItems(mlBot);
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertEquals(1, archiveContainsCount(archiveFolder.path(), "Subject: Re: \\[Closed\\] RFR: 1234: This is a pull request"));
assertEquals(1, archiveContainsCount(archiveFolder.path(), "Subject: Withdrawn: 1234: This is a pull request"));
assertEquals(1, archiveContainsCount(archiveFolder.path(), "Subject: Re: RFR: 1234: This is a pull request"));
}
}

Expand Down Expand Up @@ -1605,9 +1606,9 @@ void incrementalChanges(TestInfo testInfo) throws IOException {
assertEquals(1, updatedConversations.size());
var conversation = updatedConversations.get(0);
assertEquals(6, conversation.allMessages().size());
assertEquals("[Rev 01] RFR: This is a pull request", conversation.allMessages().get(1).subject());
assertEquals("[Rev 01] RFR: This is a pull request", conversation.allMessages().get(2).subject(), conversation.allMessages().get(2).toString());
assertEquals("[Rev 04] RFR: This is a pull request", conversation.allMessages().get(5).subject());
assertEquals("RFR: This is a pull request [v2]", conversation.allMessages().get(1).subject());
assertEquals("RFR: This is a pull request [v2]", conversation.allMessages().get(2).subject(), conversation.allMessages().get(2).toString());
assertEquals("RFR: This is a pull request [v5]", conversation.allMessages().get(5).subject());
}
}

Expand Down Expand Up @@ -1703,7 +1704,7 @@ void rebased(TestInfo testInfo) throws IOException {
assertEquals(listAddress, newMail.sender());
assertFalse(newMail.hasHeader("PR-Head-Hash"));
}
assertEquals("[Rev 01] RFR: This is a pull request", conversations.get(0).allMessages().get(1).subject());
assertEquals("RFR: This is a pull request [v2]", conversations.get(0).allMessages().get(1).subject());
}
}

Expand Down

0 comments on commit ed7df80

Please sign in to comment.