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

Zhao Song zsong at openjdk.org
Tue Jul 9 00:25:38 UTC 2024


On Mon, 8 Jul 2024 23:08:20 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotBuilder.java line 47:
>> 
>>> 45:     private IssueProject issueProject = null;
>>> 46:     private boolean ignoreStaleReviews = false;
>>> 47:     private boolean includeSimpleMerges = false;
>> 
>> I am not sure if `includeSimpleMerges` is a good name.
>> To me, if `includeSimpleMerges` is true, I think it means the bot **should** check the simple merges.
>> But actually, it means the bot should ignore simple merges?
>
> Naming is hard. Consistent naming in an established project is harder still.
> 
> `includeSimpleMerges` (or whatever it ends up being named) is supposed to work in pair with `ignoreStaleReviews`. In that pair, "include" is the opposite to "ignore": do not ignore reviews for simple merges.
> 
> That said, I'm open to better naming.

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

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

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


More information about the skara-dev mailing list