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