diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java index 15cebf3fb..eeac00179 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java @@ -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)) { @@ -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)); diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java index b77bd65ca..e5d742b0c 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java @@ -89,7 +89,7 @@ private List generateArchiveItems(List sentEmails, Repositor } } else { if (pr.state() != Issue.State.OPEN) { - threadPrefix = "FYI"; + threadPrefix = "Integrated"; } } @@ -152,7 +152,7 @@ private List generateArchiveItems(List 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 { @@ -160,10 +160,10 @@ private List generateArchiveItems(List sentEmails, Repositor } } 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); } } diff --git a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java index ccafcd731..8321918ef 100644 --- a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java +++ b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java @@ -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\\]")); } } @@ -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"); @@ -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")); } } @@ -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\\]")); } } @@ -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")); } } @@ -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()); } } @@ -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()); } }