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

Erik Joelsson erikj at openjdk.org
Fri Sep 23 17:53:12 UTC 2022

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).


Commit messages:
 - SKARA-1584

Changes: https://git.openjdk.org/skara/pull/1383/files
 Webrev: https://webrevs.openjdk.org/?repo=skara&pr=1383&range=00
  Issue: https://bugs.openjdk.org/browse/SKARA-1584
  Stats: 321 lines in 13 files changed: 270 ins; 13 del; 38 mod
  Patch: https://git.openjdk.org/skara/pull/1383.diff
  Fetch: git fetch https://git.openjdk.org/skara pull/1383/head:pull/1383

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

More information about the skara-dev mailing list