Hi everyone, Just a quick update regarding the changelog verifier. We noticed that the number of comments on some of the PRs is a bit excessive, hence as part of https://github.com/apache/lucene/pull/15409, planning to run the changelog verifier only as part of approval workflow and not explicitly commenting on PR.
So going forward, the approval workflow might show failure if the PR does not have changelog entry and skip-changelog label. And, we shouldn't see any comments regarding changelog. The changelog verifier has been a really good addition and reminder to prevent missing the changelog entry. Thanks again Stefan for this useful addition!! Regards Ankit On Tue, May 13, 2025 at 3:05 PM Ankit Jain <[email protected]> wrote: > > 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 <[email protected]> > 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 <[email protected]> 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 <[email protected]> >>> 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 <[email protected]> wrote: >>>> >>>>> Thank you Stefan! >>>>> >>>>> On Mon, May 12, 2025 at 11:09 AM Stefan Vodita < >>>>> [email protected]> 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 >>>>> >>>>
