RFR: 1584: Require re-review of a PR if the target branch changes [v2]

Erik Joelsson erikj at openjdk.org
Tue Oct 4 18:16:10 UTC 2022

On Fri, 23 Sep 2022 20:00:24 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> This change improves reviewer handling when the target branch of a PR is changed. Currently this only invalidates a review on GitHub, but not on GitLab. With this change the behavior is made uniform. In addition to this, the following new behavior is added.
>> 1. When the target branch of a PR is changed, any existing reviews in the Reviewers list will be flagged as "Re-review required (review applies to pull request targeting <branch>)" to make what happened clearer.
>> 2. If the target branch of a PR is changed back to a previously targeted branch, any reviews made against that branch may now become valid again (if all other requirements are met).
>> Note that changes to and from pr/X branches are ignored, just like before.
>> PullRequest::reviews is no longer specifying the order of reviews returned. This order was actually not consistent between implementations and I would rather we use explicit sorts when needed instead of assuming an order.
>> I'm not really happy with the placement of the new static method `calculateReviewTargetRefs`, but I couldn't really find a better location, and I really wanted to share this between different `PullRequest` implementations. I think the correct solution would be to introduce an `AbstractPullRequest` common super class, but I didn't want to do this for a single method.
>> I have verified this with the new and modified tests, as well as by running against the playground repo(s).
> Erik Joelsson has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
>  - Merge branch 'master' into SKARA-1584-rereview-targetref
>  - SKARA-1584

I believe the tests are failing due to intermittent failures in the poller. That should be fixed by https://github.com/openjdk/skara/pull/1380.


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

More information about the skara-dev mailing list