RFR: 2213: Record reviewers with stale reviews in the commit message

[Erik Joelsson]

On Thu, 28 Mar 2024 13:31:10 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java line 83:
>> 
>>> 81: 
>>> 82:         if (manualReviewersAndStaleReviewers) {
>>> 83:             reviewers.addAll(Reviewers.reviewers(currentUser, comments));
>> 
>> In the old code, if someone did a non approved verdict review and was added manually as a reviewer, they would not get reviewer credit. With this change, we unconditionally add all manual reviewers. That is probably how most people would expect the command to work though. @kevinrushforth do you have an opinion here?
>
> Yes, it seems expected that all manually credited reviewers would be added regardless of whether or not they had also done a non-approving review. I presume there will be no change in behavior in the (vast majority) of cases where the PR author has not used the `/reviewer credit` command: only approving reviewers should be added and not those who had done a non-approving review?

Correct, that case is unchanged. Since you agree with the new behavior, I'm also ok with it. I did find the old one a bit odd.

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

PR Review Comment: https://git.openjdk.org/skara/pull/1626#discussion_r1543272476