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

Pavel Rappo prappo at openjdk.org
Mon Jul 8 23:21:11 UTC 2024


On Mon, 8 Jul 2024 22:32:38 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> If a repository is configured to ignore stale reviews, every commit to a PR made against that repo means that the PR needs to be re-reviewed. Re-reviewing is costly, and not all commits are worth that cost.
>> 
>> One example of such a commit is a simple merge. During a PR's lifetime, the PR's target branch (typically, repo's `master`) might be merged into the PR multiple times. Usually, such merges are trivial and would effectively be performed by Skara automatically if the PR didn't have them in the first place [^*]. For such commits, it makes sense to not require review.
>> 
>> This is my first contribution to Skara domain logic, which I've just started learning about. Also, while I tried to code this change according to Skara customs and idioms, I might have missed something. I guess what I'm saying is **take extra care** reviewing this PR. Thanks.
>> 
>> 
>> [^*]: This is the main assumption of this PR. Any simple merges present in a PR change the end result of that PR insignificantly, if at all.
>
> vcs/src/main/java/org/openjdk/skara/vcs/git/GitRepository.java line 1012:
> 
>> 1010:         }
>> 1011:     }
>> 1012: 
> 
> Maybe it's better to add an interface in `ReadOnlyRepository`

If we add this method to `ReadOnlyRepository`, then we'll need to provide some implementation of it for `HgRepository`, which is a subclass of `ReadOnlyRepository`. Not only does `HgRepository` need no such functionality (AFAIK, PRs are not modelled to work in Mercurial repos), a Mercurial implementation will likely be more involved than that of Git.

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

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


More information about the skara-dev mailing list