RFR: 1588: Bridge messages should not be sent for PRs in draft state [v2]

Guoxiong Li gli at openjdk.org
Tue Feb 14 09:01:33 UTC 2023

On Mon, 13 Feb 2023 19:15:54 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

> Were you able to run the manual tests to verify the new methods on PullRequest?

I had run the manual tests locally. Everything looks fine.

> bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveWorkItem.java line 144:
>> 142:         // after the last converted time should be ignored.
>> 143:         if (pr.isDraft()) {
>> 144:             var lastDraftTime = pr.lastMarkedAsDraftTime();
> This method gets called for every comment on the PR, every time `ArchiveWorkItem` is run for a PR. The result of `pr.lastMarkedAsDraftTime()` needs to be retrieved once and reused.

I move `pr.lastMarkedAsDraftTime()` to the method `ArchiveWorkItem#run` to avoid the duplicated invocation.

> forge/src/main/java/org/openjdk/skara/forge/github/GitHubPullRequest.java line 712:
>> 710:                 .filter(obj -> obj.contains("event"))
>> 711:                 .filter(obj -> obj.get("event").asString().equals("convert_to_draft"))
>> 712:                 .reduce((a, b) -> b)
> Are you sure of the order events are returned in? I couldn't find a definition of the order. Maybe safer to map to date and reduce to the highest one?

I can't find the related document too. I tried the rest api to get the related data and found the order personally. Now I adjust the code to find the highest one to avoid the API result changes in the future.

> forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabMergeRequest.java line 792:
>> 790:                 .filter(obj -> obj.get("system").asBoolean())
>> 791:                 .filter(obj -> draftMessage.equals(obj.get("body").asString()))
>> 792:                 .reduce((a, b) -> a)
> Same here as for GitHub, maybe reduce to highest date just to be safe?

Same as above and also adjust it.


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

More information about the skara-dev mailing list