RFR: 2331: Improve review notes

[Pavel Rappo]

On Thu, 18 Jul 2024 18:08:29 GMT, Chen Liang <liach 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
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 616:
> 
>> 614:                                                            + "](" + pr.filesUrl(hash.get()) + "))";
>> 615:                                                } else {
>> 616:                                                    entry += " ⚠️ Review applies to [" + hash.get().abbreviate()
> 
> Now that 🔄 and ⚠️ can appear on 2 different reviewers in the same PR, we should consider changing this warning symbol to some other emoji that indicates "outdated".

Rest assured, I will not integrate this PR until we figure out what to do with the issue you raised, Chen. That issue should be fixed either in this PR or a follow-up one.

Personally, I think if a review is stale but otherwise good, we don't need to add any emoji. It suffices to add the commit hash the review applies to, and maybe indicate how far away that commit is from HEAD, to quantify staleness: for example, "3 commits behind". Any extra commits added since that review may only be simple merges anyway.

However, if a review is stale and restricted (i.e. does not count in required reviews), we may add "⚠️". In a sense, it would provide a similar indication to that of a repo configured with:

    useStaleReviews=true
    acceptSimpleMerges=false

In this picture, the "🔄" emoji goes away completely.

-------------

PR Review Comment: https://git.openjdk.org/skara/pull/1679#discussion_r1684158168