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.

Sign up for GitHub

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

410: Update mailing list bridge subject decorations #662

Closed
wants to merge 1 commit into from
Closed

410: Update mailing list bridge subject decorations #662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 5 additions & 5 deletions bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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