RFR: 1853: Make it possible to disable merge PRs for a repository [v3]

Erik Joelsson erikj at openjdk.org
Tue Apr 11 18:43:28 UTC 2023


On Tue, 11 Apr 2023 18:29:16 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java line 263:
>> 
>>> 261:             return List.of();
>>> 262:         }
>>> 263:         removeErrorComment(mergeDisabledText, comments);
>> 
>> I think this whole check should be moved inside the `currentCheckValid` block. The check is cheap, but I still think it should be grouped with other similar checks.
>> 
>> I don't think we should remove error comments. None of the other similar comments are removed, so unless we want to change that, then this new comment shouldn't be either.
>
> I thought if we put the check on the top of the method, it would help us save some rest api calls. I am also ok with moving it inside the currentCheckValid block.

That is true, we would. However, since this is such a rare code path, it's not really worth optimizing for. I think it's more important that the code is more readable.

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

PR Review Comment: https://git.openjdk.org/skara/pull/1499#discussion_r1163195945


More information about the skara-dev mailing list