RFR: 1587: Adding review comments should not mean approval status change
Zhao Song
zsong at openjdk.org
Wed Oct 19 19:29:13 UTC 2022
On Wed, 19 Oct 2022 19:22:42 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.
bots/pr/src/test/java/org/openjdk/skara/bots/pr/ReviewersTests.java line 658:
> 656: // The pr should still contain 'Ready' label
> 657: updatedPr = author.pullRequest(pr.id());
> 658: assertTrue(updatedPr.labelNames().contains("ready"));
I am still wondering why we should update the pr manually.
-------------
PR: https://git.openjdk.org/skara/pull/1399
More information about the skara-dev
mailing list