RFR: 2331: Improve review notes [v4]

Erik Joelsson erikj at openjdk.org
Mon Aug 19 17:05:10 UTC 2024


On Mon, 19 Aug 2024 16:22:25 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
> 
>  - Respond to feedback
>  - Merge branch 'master' into 2331
>  - Don't special-case wrong target or missing commit
>    
>    After out-of-band conversation with @zhaosongzs, we decided to drop
>    useStaleReviews check.
>  - Avoid emojis and "required" when a PR is ready
>  - Re-indent code
>  - Don't add "Re-review required" if reviewed
>    
>    If review requirements are fully satisfied, don't add "Re-review
>    required (review applies to ..." for stale reviews; instead, add
>    "Review applies to ..."
>  - Don't add "Review applies to" to the head of a PR

I'm ok with the review notes here, just a minor nit for code readability.

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

PR Review: https://git.openjdk.org/skara/pull/1679#pullrequestreview-2246045393


More information about the skara-dev mailing list