RFR: 1587: Adding review comments should not mean approval status change

[Erik Joelsson]

On Wed, 19 Oct 2022 19:24:18 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> The issue here is ' if a user leaves additional comments using the "Review changes" dialog, and selects the "Comments", then their approval is considered revoked.'
>> 
>> I believe this happens because if we use "Review changes" to add a comment, it will add a review with `Review.Verdict.NONE` to our pr.  According to our current logic, this new review will cover old reviews, so the approval will be revoked.
>> 
>> I made changes to `CheckablePullRequest#filterActiveReviews`. Now If latest review is just a comment, the verdict will inherit from the previous review.
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java line 155:
> 
>> 153:                                 review.body().isPresent() ? review.body().get() : null,
>> 154:                                 review.targetRef());
>> 155:                     }
> 
> I don't whether it is a good idea to create a new review here.
> 
> If we don't want to create a new review, we should make `verdict` not final and add a set method in Review.

I don't think we should create a new `Review` here. I _think_ that the way to achieve the desired behavior is to ignore reviews where the verdict isn't `APPROVED` or `DISAPPROVED`. A `Review` with verdict `NONE` is essentially pointless when considering active reviews.

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

PR: https://git.openjdk.org/skara/pull/1399