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

Zhao Song zsong at openjdk.org
Tue Jul 9 02:03:40 UTC 2024


On Mon, 8 Jul 2024 23:28:30 GMT, Pavel Rappo <prappo 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.
>
> Pavel Rappo has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Respond to feedback

bots/pr/src/main/java/org/openjdk/skara/bots/pr/ReviewCoverage.java line 62:

> 60:         if (!includeSimpleMerges) {
> 61:             return false;
> 62:         }

But here, if `includeSimpleMerges==true`, this method will try to ignore the simple merges.
if `includeSimpleMerges==false`, this method will not ignore the simple merges

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

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


More information about the skara-dev mailing list