RFR: 2312: Do not require re-review for a simple merge [v4]

[Erik Joelsson]

On Wed, 10 Jul 2024 10:15:19 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> I agree with Zhao's concern about the naming being confusing. I understand the motivation given the existing `ignoreStaleReviews`, which is almost equally bad. Just the other day, we reflected over that config option when we enabled it for the mainline jdk repository as it could be interpreted both ways, but we agreed that the opposite interpretation was our instinctive assumption.
>> 
>> Perhaps we should just fix both names while we are at it? Can do the existing one in a separate fix if you prefer, but either way works for me. When deploying it, we will need to take care to update the bot configuration files. This kind of change isn't without precedent and something we can do.
>> 
>> In that case I would suggest the following, but open for suggestions:
>> 
>> - `useStaleReviews`
>> - `acceptSimpleMerges`
>
>> Just the other day, we reflected over that config option when we enabled it for the mainline jdk repository as it could be interpreted both ways, but we agreed that the opposite interpretation was our instinctive assumption.
> 
> When I started this work, I was surprised to realise that `ignoreStaleReviews` is true means that outdated reviews are **not** included in the total count of reviews. Like you said, it's counterintuitive.
> 
> But I didn't propose to change it because it's an established property that some repos already use. So, like you said, if we change it, we should be prepared to change the configuration of the affected repos too.
> 
> Those names you suggested, are they both for JSON configuration and Java code? In other words, should I change code like this?
> 
>             if (repo.value().contains("useStaleReviews")) {
>                 botBuilder.useStaleReviews(repo.value().get("useStaleReviews").asBoolean());
>             }
>             if (repo.value().contains("acceptSimpleMerges")) {
>                 botBuilder.acceptSimpleMerges(repo.value().get("acceptSimpleMerges").asBoolean());
>             }

Yes, I would like it to be consistent.

-------------

PR Review Comment: https://git.openjdk.org/skara/pull/1672#discussion_r1672198351