RFR: 1584: Require re-review of a PR if the target branch changes [v3]
Erik Joelsson
erikj at openjdk.org
Fri Oct 7 18:11:26 UTC 2022
On Fri, 7 Oct 2022 09:23:01 GMT, Erik Helin <ehelin at openjdk.org> wrote:
>> Erik Joelsson has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
>>
>> - Merge branch 'master' into SKARA-1584-rereview-targetref
>> - Merge branch 'master' into SKARA-1584-rereview-targetref
>> - SKARA-1584
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 505:
>
>> 503: if (!review.targetRef().equals(pr.targetRef())) {
>> 504: entry += " 🔄 Re-review required (review applies to pull request targeting [" + review.targetRef()
>> 505: + "](" + pr.repository().webUrl(new Branch(review.targetRef())) + "))";
>
> I would probably have phrased this as `"Re-review required (review was made when pull request targeted the " + review.targetRef() + " branch)";`, but that is just my style 😸
Yes, that reads better, but I will keep the link.
> forge/src/main/java/org/openjdk/skara/forge/Review.java line 52:
>
>> 50: }
>> 51:
>> 52: public Review(Review other, String targetRef) {
>
> I would probably have used a static method like `public static Review withTargetRef(Review r, String targetRef)` or potentially have a instance method `public Review withTargetRef(String targetRef)`.
I like the instance method.
-------------
PR: https://git.openjdk.org/skara/pull/1383
More information about the skara-dev
mailing list