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