RFR: 2331: Improve review notes [v3]
Kevin Rushforth
kcr at openjdk.org
Fri Aug 2 19:33:11 UTC 2024
On Thu, 1 Aug 2024 10:06:20 GMT, Pavel Rappo <prappo at openjdk.org> wrote:
>>> If there are enough reviewers, no emoji or the word "required" should be used.
>>
>> I think I now understand what you are trying to do, although I don 't think I agree with the premise. If I understand the logic (which I think I do now), here are two cases that won't do what I would expect:
>>
>> 1. `useStaleReviews=false` : if there are too few reviewers, simple merges will show a warning icon
>> 2. `useStaleReviews=true` : if there are enough reviewers, all other reviews will not get the warning icon
>>
>> I think 2 basically makes the warning icon useless (and 1 is a bug)
>
> @kevinrushforth ping
Sorry for the delay.
For the (now largely unused) case of `useStaleReviews=true`, I would expect no change from today's behavior: Regardless of whether or not the review criteria is met, print "⚠️ Review applies to HASH" for all stale reviews.
For the (now widely-used) case of `useStaleReviews=false`: If the review criteria is _not_ met, print "🔄 Re-review required (review applies to HASH)" for all stale reviews; if the review criteria _is_ met, print "Review applies to HASH" with no emoji for all stale reviews.
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1679#discussion_r1702246422
More information about the skara-dev
mailing list