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

[Erik Joelsson]

On Tue, 9 Jul 2024 00:32:12 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> So you said `includeSimpleMerges` means "do not ignore reviews for simple merges."?
>
> Yes.

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`

>> Yes, for `HgRepository`,  you just need to put `throw new UnsupportedOperationException();` in the implementation
>
> Here's what I'm going to do:
> 
>   - pull up new `Commits commits(List<Hash> reachableFrom, List<Hash> unreachableFrom)` to `ReadOnlyRepository`,
>   - pull up `boolean isRemergeDiffEmpty(Hash mergeCommitHash)` to `Repository`.
> 
> `isRemergeDiffEmpty` is not a good fit for `ReadOnlyRepository` as far as I understand it. Additionally, if in the future we decide to implement `--remerge-diff` manually, for whatever reason, it'll be even less fit.

I agree with Zhao, the API should be on the interface. It's fine to just throw unsupported from HG.

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

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