RFR: 1153: Some emails still not posted as comments in PR [v2]

[Erik Joelsson]

On Thu, 2 Sep 2021 22:43:11 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

> Is this something you can test before deploying?

With a local edit of the BotRunner, I have manually tested the MailingListArchiveReaderBot and ArchiveReaderWorkItem by running them with the full production configuration, but prevented any other WorkItems from actually running. I've then inspected the set of CommentPosterWorkItems that get scheduled.

> bot/src/main/java/org/openjdk/skara/bot/BotRunner.java line 254:
> 
>> 252:                     for (var pendingItem : pending.entrySet()) {
>> 253:                         // If there are pending items of the same type that we cannot run concurrently with, replace them.
>> 254:                         if (item.replaces(pendingItem.getKey())) {
> 
> I presume that `a.concurrentWith(b) == b.concurrentWith(a)` for all work items (meaning that `concurrentWith` is a commutative operation)? The reason I ask is that the old code checked `pendingItem.getKey().concurrentWith(item)` while the new code checks `item.concurrentWith(pendingItem.getKey()))`. I expect it is fine.

As far as I can tell, it's commutative. I flipped it around because it made more sense to me to ask "replaces?" that way, but who calls who really shouldn't matter in either case.

> mailinglist/src/main/java/org/openjdk/skara/mailinglist/Mbox.java line 101:
> 
>> 99:             for (var email : emailsToCheck) {
>> 100:                 if (email.id().toString().contains("<VtLlAA6xMBK-6Ib-4SbkYZJssOuyqPl5Dr7kdiJUf6A=.9c549bc6-867f-4ba6-8727-efafe7cc18e2 at github.com>")) {
>> 101:                     log.warning("Found first email of jdk#5300 in mbox");
> 
> What is the purpose of this check?

Ah, that's a left over debug message (and break point line) from my testing. Will remove.

-------------

PR: https://git.openjdk.java.net/skara/pull/1214