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