RFR: 1594: Convert rest of bots polling PRs and Issues to use new pollers

Erik Joelsson erikj at openjdk.org
Tue Nov 15 22:05:25 UTC 2022

On Mon, 7 Nov 2022 09:23:28 GMT, Erik Helin <ehelin at openjdk.org> wrote:

> I feel like I'm always comment on naming, feel free to ignore such suggestions, they are just suggestions 😃 In this case I would have renamed `PullRequestPoller.lastBatchHandled` to `PullRequestPoller.ignore(List<PullRequest> prs)` and the bots would typically call `poller.ignore(prs);`

I'm not completely satisfied with the naming of the methods either, but I don't really get the "ignore" suggestion here. The idea with the method is to acknowledge to the poller that what it returned has now been accepted. In most cases, having this two step ack protocol isn't needed, but in some bots, there are further remote calls made in `getPeriodicItems` after `updatedPullRequests` is called, which could potentially fail, and if they do, I want to make sure the next call to `getPeriodicItems` will include those PRs again. I'm sure you figured this out already.

In a separate discussion, you also pointed out that the naming of the `retryPullRequest` methods was strange to you. You may have a point there too. There is also an undocumented requirement in the PullRequestPoller, that `retryPullRequest` must not be called before `lastBatchHandled`. This is how I implicitly assumed it was always going to be used, but it does need to be made explicit, at least in documentation. I will look into this.


PR: https://git.openjdk.org/skara/pull/1412

More information about the skara-dev mailing list