RFR: 2331: Improve review notes [v3]
Kevin Rushforth
kcr at openjdk.org
Wed Jul 24 14:15:23 UTC 2024
On Tue, 23 Jul 2024 18:53:19 GMT, Pavel Rappo <prappo at openjdk.org> wrote:
>> There are two variants of a note that can be added to an item in the "Reviewers" list of a PR body:
>>
>> - 🔄 Re-review required ...
>> - ⚠️ Review applies to ...
>>
>> That note may confuse or draw unneeded attention to the PR body.
>>
>> Firstly, as a result of recently integrated [SKARA-2312][], "⚠️ Review applies to ..." is also added to an item (i.e. review) that applies to the head of the PR. Secondly, "🔄 Re-review required (review applies to ..." may be present for some items even though a PR has the green "ready" label.
>>
>> [SKARA-2312]: https://bugs.openjdk.org/browse/SKARA-2312
>
> Pavel Rappo has updated the pull request incrementally with one additional commit since the last revision:
>
> Don't special-case wrong target or missing commit
>
> After out-of-band conversation with @zhaosongzs, we decided to drop
> useStaleReviews check.
bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 618:
> 616: if (!hash.get().equals(pr.headHash())) {
> 617: if (reviewCoverage.covers(review) || !tooFewReviewers) {
> 618: entry += (tooFewReviewers ? " ⚠️ " : " ")
Why are you using `tooFewReviewers` to decide whether or not to show the warning emoji. Shouldn't this check be `useStaleReviews`?
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1679#discussion_r1689861341
More information about the skara-dev
mailing list