Hey all, I started poking around at Github actions on my fork. https://github.com/mumrah/kafka/actions
I'll post a PR if I get it working and we can discuss what kind of settings we want (or if we want it all) -David On Tue, Jun 6, 2023 at 1:18 PM Chris Egerton <chr...@aiven.io.invalid> wrote: > Hi Josep, > > Thanks for bringing this up! Will try to keep things brief. > > I'm generally in favor of this initiative. A couple ideas that I really > liked: requiring a component label (producer, consumer, connect, streams, > etc.) before closing, and disabling auto-close (i.e., automatically tagging > PRs as stale, but leaving it to a human being to actually close them). > > We might replace the "stale" label with a "close-by-<date>" label so that > it becomes even easier for us to find the PRs that are ready to be closed > (as opposed to the ones that have just been labeled as stale without giving > the contributor enough time to respond). > > I've also gone ahead and closed some of my stale PRs. Others I've > downgraded to draft to signal that I'd like to continue to pursue them, but > have to iron out merge conflicts first. For the last ones, I'll ping for > review. > > One question that came to mind--do we want to distinguish between regular > and draft PRs? I'm guessing not, since they still add up to the total PR > count against the project, but since they do also implicitly signal that > they're not intended for review (yet) it may be friendlier to leave them > up. > > Cheers, > > Chris > > On Tue, Jun 6, 2023 at 10:18 AM Mickael Maison <mickael.mai...@gmail.com> > wrote: > > > Hi Josep, > > > > Thanks for looking into this. This is clearly one aspect where we need > > to improve. > > > > We had a similar initiative last year > > (https://lists.apache.org/thread/66yj9m6tcyz8zqb3lqlbnr386bqwsopt) and > > we closed many PRs. Unfortunately we did not follow up with a process > > or automation and we are back to the same situation. > > > > Manually reviewing all these PRs is a huge task, so I think we should > > at least partially automate it. I'm not sure if we should manually > > review the oldest PRs (pre 2020). There's surely many interesting > > things but I wonder if we should instead focus on the more recent ones > > as they have a higher chance of 1) still making sense, 2) getting > > updates from their authors, 3) needing less rebasing. If something has > > been broken since 2016 but we never bothered to fix the PR it means it > > can't be anything critical! > > > > Finally as Colin mentioned, it looks like a non negligible chunk of > > stale PRs comes from committers and regular contributors. So I'd > > suggest we each try to clean our own backlog too. > > > > I wonder if we also need to do something in JIRA. Querying for > > unresolved tickets returns over 4000 items. Considering we're not > > quite at KAFKA-15000 yet, that's a lot. > > > > Thanks, > > Mickael > > > > > > On Tue, Jun 6, 2023 at 11:35 AM Josep Prat <josep.p...@aiven.io.invalid> > > wrote: > > > > > > Hi Devs, > > > I would say we can split the problem in 2. > > > > > > Waiting for Author feedback: > > > We could have a bot that would ping authors for the cases where we have > > PRs > > > that are stalled and have either: > > > - Merge conflict > > > - Unaddressed reviews > > > > > > Waiting for reviewers: > > > For the PRs where we have no reviewers and there are no conflicts, I > > think > > > we would need some human interaction to determine modules (maybe this > can > > > be automated) and ping people who could review. > > > > > > What do you think? > > > > > > Best, > > > > > > On Tue, Jun 6, 2023 at 11:30 AM Josep Prat <josep.p...@aiven.io> > wrote: > > > > > > > Hi Nikolay, > > > > > > > > With a bot it will be complicated to determine what to do when the PR > > > > author is waiting for a reviewer. If a person goes over them, can > > check if > > > > they are waiting for reviews and tag the PR accordingly and maybe > ping > > a > > > > maintainer. > > > > If you look at my last email I described a flow (but AFAIU it will > work > > > > only if a human executes it) where the situation you point out would > be > > > > covered. > > > > > > > > ——— > > > > Josep Prat > > > > > > > > Aiven Deutschland GmbH > > > > > > > > Alexanderufer 3-7, 10117 Berlin > > > > > > > > Amtsgericht Charlottenburg, HRB 209739 B > > > > > > > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen > > > > > > > > m: +491715557497 > > > > > > > > w: aiven.io > > > > > > > > e: josep.p...@aiven.io > > > > > > > > On Tue, Jun 6, 2023, 11:20 Николай Ижиков <nizhi...@apache.org> > wrote: > > > > > > > >> Hello. > > > >> > > > >> What is actions of contributor if no feedback given? [1], [2] > > > >> > > > >> [1] https://github.com/apache/kafka/pull/13278 > > > >> [2] https://github.com/apache/kafka/pull/13247 > > > >> > > > >> > 2 июня 2023 г., в 23:38, David Arthur <mum...@gmail.com> > > написал(а): > > > >> > > > > >> > I think this is a great idea. If we don’t want the auto-close > > > >> > functionality, we can set it to -1 > > > >> > > > > >> > I realize this isn’t a vote, but I’m +1 on this > > > >> > > > > >> > -David > > > >> > > > > >> > On Fri, Jun 2, 2023 at 15:34 Colin McCabe <cmcc...@apache.org> > > wrote: > > > >> > > > > >> >> That should read "30 days without activity" > > > >> >> > > > >> >> (I am assuming we have the ability to determine when a PR was > last > > > >> updated > > > >> >> on GH) > > > >> >> > > > >> >> best, > > > >> >> Colin > > > >> >> > > > >> >> On Fri, Jun 2, 2023, at 12:32, Colin McCabe wrote: > > > >> >>> Hi all, > > > >> >>> > > > >> >>> Looking at GitHub, I have a bunch of Kafka PRs of my own that > I've > > > >> >>> allowed to become stale, and I guess are pushing up these > numbers! > > > >> >>> Overall I support the goal of putting a time limit on PRs, just > so > > > >> that > > > >> >>> we can focus our review bandwidth. > > > >> >>> > > > >> >>> It may be best to start with a simple approach where we mark PRs > > as > > > >> >>> stale after 30 days and email the submitter at that time. And > then > > > >> >>> delete after 60 days. (Of course the exact time periods might be > > > >> >>> something gother than 30/60 but this is just an initial > > suggestion) > > > >> >>> > > > >> >>> best, > > > >> >>> Colin > > > >> >>> > > > >> >>> > > > >> >>> On Fri, Jun 2, 2023, at 00:37, Josep Prat wrote: > > > >> >>>> Hi all, > > > >> >>>> > > > >> >>>> I want to say that in my experience, I always felt better as a > > > >> >> contributor > > > >> >>>> when a person told me something than when a bot did. That being > > said, > > > >> >> I'm > > > >> >>>> not against bots, and I think they might be a great solution > > once we > > > >> >> have a > > > >> >>>> manageable number of open PRs. > > > >> >>>> > > > >> >>>> Another great question that adding a bot poses is types of > > staleness > > > >> >>>> detection. How do we distinguish between staleness from the > > author's > > > >> >> side > > > >> >>>> or from the lack of reviewers/maintainers' side? That's why I > > started > > > >> >> with > > > >> >>>> a human approach to be able to distinguish between these 2 > cases. > > > >> Both > > > >> >>>> situations should have different messages and actually > different > > > >> >> intended > > > >> >>>> receivers. In case of staleness because of author inactivity, > the > > > >> >> message > > > >> >>>> should encourage the author to update the PR with the requested > > > >> changes > > > >> >> or > > > >> >>>> resolve the conflicts. But In many cases, PRs are stale because > > of > > > >> lack > > > >> >> of > > > >> >>>> reviewers. This would need a different message, targeting > > > >> maintainers. > > > >> >>>> > > > >> >>>> Ideally (with bot or not) I believe the process should be as > > follows: > > > >> >>>> - Check PRs that are stale. > > > >> >>>> - See if they have labels, if not add proper labels (connect, > > core, > > > >> >>>> clients...) > > > >> >>>> - PR has merge conflicts > > > >> >>>> -- Merge conflicts exist and target files that still exist, > ping > > the > > > >> >> author > > > >> >>>> asking for conflict resolution and add some additional label > like > > > >> >> `stale`. > > > >> >>>> -- Merge conflicts exist and target files that do not exist > > anymore, > > > >> let > > > >> >>>> the author know that this PR is obsolete, label the PR as > > 'obsolete' > > > >> and > > > >> >>>> close the PR. > > > >> >>>> - PR is mergeable, check whose action is needed (author or > > reviewers) > > > >> >>>> -- Author: let the author know that there are pending comments > to > > > >> >> address. > > > >> >>>> Add some additional label, maybe `stale` again > > > >> >>>> -- Reviewer: ping some reviewers that have experience or lately > > > >> touched > > > >> >>>> this piece of the codebase, add a label `reviewer needed` or > > > >> something > > > >> >>>> similar > > > >> >>>> - PRs that have `stale` label after X days, will be closed. > > > >> >>>> > > > >> >>>> Regarding the comments about only committers and collaborators > > being > > > >> >> able > > > >> >>>> to label PRs, this is true, not everyone can do this. However, > > this > > > >> >> could > > > >> >>>> be a great opportunity for the newly appointed contributors to > > > >> exercise > > > >> >>>> their new permissions :) > > > >> >>>> > > > >> >>>> Let me know if it makes sense to you all. > > > >> >>>> > > > >> >>>> Best, > > > >> >> > > > >> > -- > > > >> > David Arthur > > > >> > > > >> > > > > > > -- > > > [image: Aiven] <https://www.aiven.io> > > > > > > *Josep Prat* > > > Open Source Engineering Director, *Aiven* > > > josep.p...@aiven.io | +491715557497 > > > aiven.io <https://www.aiven.io> | < > > https://www.facebook.com/aivencloud> > > > <https://www.linkedin.com/company/aiven/> < > > https://twitter.com/aiven_io> > > > *Aiven Deutschland GmbH* > > > Alexanderufer 3-7, 10117 Berlin > > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen > > > Amtsgericht Charlottenburg, HRB 209739 B > > > -- -David
