RFR: 1588: Bridge messages should not be sent for PRs in draft state [v2]
Erik Joelsson
erikj at openjdk.org
Mon Feb 13 19:18:14 UTC 2023
On Sat, 11 Feb 2023 12:19:48 GMT, Guoxiong Li <gli at openjdk.org> wrote:
>> Hi all,
>>
>> This patch prevents the `mailling bridge bot` from sending emails when a PR is draft.
>> And the related test case is added.
>>
>> Thanks for taking the time to review.
>>
>> Best Regards,
>> -- Guoxiong
>
> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>
> Filter the comments in method ArchiveWorkItem#ignoreComment
This looks pretty good, but see comments. Were you able to run the manual tests to verify the new methods on PullRequest?
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.
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?
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?
-------------
PR: https://git.openjdk.org/skara/pull/1469
More information about the skara-dev
mailing list