> I thought of making it an approval step, but that seemed too forceful. There are good arguments for it though. Maybe we run the bot as is for a couple weeks, see how we like it, and change it if we think an approval step would be better?
IMO having it as an approval step is not at all forceful. Either you eventually want to add changelog (failing step serves as reminder) or not (just add skip-changelog label). So, it avoids potentially unnecessary comments on PR without needing the author to add changelog entry or label at any specific time in the lifetime of PR. That being said, I am aligned with the suggestion of running as is for a couple weeks. > I slightly prefer it being explicit, but I don't feel strongly about it. If I have to add it to a PR, I'm counting on auto-complete to help! :) I prefer skip-changelog slightly as the intent is to skip changelog entry, not the changelog check workflow. But really minor! :) On Tue, May 13, 2025 at 2:53 PM Stefan Vodita <stefan.vod...@gmail.com> wrote: > Thank you Ankit! I appreciate the observations, and yes, the workflow does > also label PRs with milestones, so hopefully that would have helped with > the PR you mentioned. > > > Instead of commenting, should it be a step in the approval workflow? > Adding the skip label indicates explicit intent, and should make the > approval step succeed. > > I thought of making it an approval step, but that seemed too forceful. > There are good arguments for it though. Maybe we run the bot as is for a > couple weeks, see how we like it, and change it if we think an approval > step would be better? > > > skip-changelog-check is slightly verbose > > I slightly prefer it being explicit, but I don't feel strongly about it. > If I have to add it to a PR, I'm counting on auto-complete to help! :) > > On Tue, 13 May 2025 at 22:45, Ankit Jain <jain.ank...@gmail.com> wrote: > >> Just noticed that the logic for adding target milestones using >> the changelog entry is already there. Nice, thanks! >> >> Created small PR for updating the skip-changelog label string - >> https://github.com/apache/lucene/pull/14661 >> >> On Tue, May 13, 2025 at 2:12 PM Ankit Jain <jain.ank...@gmail.com> wrote: >> >>> Thanks Stefan for adding this very useful workflow. Few observations: >>> >>> - Instead of commenting, should it be a step in the approval >>> workflow? Adding the skip label indicates explicit intent, and should >>> make >>> the approval step succeed. >>> - Really minor: skip-changelog-check is slightly verbose. Wondering >>> if *skip-changelog *is good enough >>> - Will be great to validate changelog entry against the target >>> milestone. I was careless enough to miss that for one of the PRs - >>> https://github.com/apache/lucene/pull/14609 >>> >>> With these changes, we don't need to worry about multiple changelog >>> comments in PR. For example - I don't want to skip-changelog, but not add >>> changelog until the PR is almost ready to be merged. Then, I need to add a >>> skip-changelog label first and then remove that later. >>> >>> >>> - Ankit >>> >>> On Mon, May 12, 2025 at 4:55 AM Adrien Grand <jpou...@gmail.com> wrote: >>> >>>> Thank you Stefan! >>>> >>>> On Mon, May 12, 2025 at 11:09 AM Stefan Vodita <stefan.vod...@gmail.com> >>>> wrote: >>>> >>>>> Hi everyone, >>>>> >>>>> We've been working on a bot to look at PRs, check for a CHANGES entry ( >>>>> #13898 <https://github.com/apache/lucene/issues/13898>), and >>>>> determine the milestone the PR belongs to (#14190 >>>>> <https://github.com/apache/lucene/issues/14190>). >>>>> >>>>> I got a few approvals to enable this bot in #14644 >>>>> <https://github.com/apache/lucene/pull/14644>, but I want to tell you >>>>> all what to expect. If you want to see an example, look at the PR >>>>> I've been testing on <https://github.com/stefanvodita/lucene/pull/9>. >>>>> 1. If a PR does not have a CHANGES entry, the bot should post a >>>>> comment reminding us to create one. >>>>> 2. If a PR has a CHANGES entry, the bot will try to find the release >>>>> that entry is under and add the corresponding milestone to the PR. >>>>> >>>>> I'll keep an eye out on the bot, but if this isn't the behaviour >>>>> you're seeing, it would be useful to know! It won't work perfectly from >>>>> the >>>>> start, but I think at this stage the bot should be able to handle most PRs >>>>> correctly. I'm planning to enable the bot tomorrow. >>>>> >>>>> Stefan >>>>> >>>>> >>>> >>>> -- >>>> Adrien >>>> >>>
