Thanks for the investigation, Luke! I think just pining committer's ids in a comment is sufficient.
On Sun, Feb 13, 2022 at 5:00 AM Luke Chen <show...@gmail.com> wrote: > Hi Guozhang, > > Had a quick search on the github action, didn't find any notification > related actions. > However, there's a github action to auto leave a comment before closing > PR.[1] > So, I think at least, we can leave a comment that notify the PR > participants. > If the auto comment action can support mention users (i.e. @ user), we can > notify anyone we want. > > [1] https://github.com/actions/stale#close-pr-message > > Thanks. > Luke > > > On Fri, Feb 11, 2022 at 6:36 AM Guozhang Wang <wangg...@gmail.com> wrote: > > > Just going back to the PRs, @David Jacot, do you know if the > actions/stale > > <https://github.com/actions/stale> tool is able to send notifications to > > pre-configured recipients when closing stale PRs? > > > > On Wed, Feb 9, 2022 at 9:21 PM Matthias J. Sax <mj...@mailbox.org.invalid > > > > wrote: > > > > > Nikolay, > > > > > > thanks for helping out! > > > > > > > First, I thought it’s an author job to keep KIP status up to date. > > > > But, it can be tricky to determine actual KIP status because of lack > of > > > feedback from committers > > > > > > Yes, it is the author's task, but it's also the author's task to keep > > > the discussion alive (what -- to be fair -- can be hard). We had some > > > great contributions thought that took very long, but the KIP author > kept > > > following up and thus signaling that they still have interest. Just > > > going silent and effectively dropping a KIP is not the best way (even > if > > > I can understand that it sometime frustrating and some people just walk > > > away). > > > > > > > > > > Second - the other issue is determine - what KIP just wait for a hero > > to > > > implement it, and what just wrong idea or something similar. > > > > > > As pointed out on the KIP wiki page, if somebody is not willing to > > > implement the KIP, they should not even start it. Getting a KIP voted > > > but not finishing it, is not really helping the project. > > > > > > About "just the wrong idea": this also happens, but usually it's clear > > > quite quickly if people raise concerns about an idea. > > > > > > > > > -Matthias > > > > > > > > > On 2/9/22 12:13, Nikolay Izhikov wrote: > > > >> Thanks for that list, Nikolay, > > > > > > > > Thank, John. > > > > > > > > I made a second round of digging through abandoned PR’s. > > > > Next pack, that should be closed: > > > > > > > > https://github.com/apache/kafka/pull/1291 > > > > https://github.com/apache/kafka/pull/1323 > > > > https://github.com/apache/kafka/pull/1412 > > > > https://github.com/apache/kafka/pull/1757 > > > > https://github.com/apache/kafka/pull/1741 > > > > https://github.com/apache/kafka/pull/1715 > > > > https://github.com/apache/kafka/pull/1668 > > > > https://github.com/apache/kafka/pull/1666 > > > > https://github.com/apache/kafka/pull/1661 > > > > https://github.com/apache/kafka/pull/1626 > > > > https://github.com/apache/kafka/pull/1624 > > > > https://github.com/apache/kafka/pull/1608 > > > > https://github.com/apache/kafka/pull/1606 > > > > https://github.com/apache/kafka/pull/1582 > > > > https://github.com/apache/kafka/pull/1522 > > > > https://github.com/apache/kafka/pull/1516 > > > > https://github.com/apache/kafka/pull/1493 > > > > https://github.com/apache/kafka/pull/1473 > > > > https://github.com/apache/kafka/pull/1870 > > > > https://github.com/apache/kafka/pull/1883 > > > > https://github.com/apache/kafka/pull/1893 > > > > https://github.com/apache/kafka/pull/1894 > > > > https://github.com/apache/kafka/pull/1912 > > > > https://github.com/apache/kafka/pull/1933 > > > > https://github.com/apache/kafka/pull/1983 > > > > https://github.com/apache/kafka/pull/1984 > > > > https://github.com/apache/kafka/pull/2017 > > > > https://github.com/apache/kafka/pull/2018 > > > > > > > >> 9 февр. 2022 г., в 22:37, John Roesler <vvcep...@apache.org> > > > написал(а): > > > >> > > > >> Thanks for that list, Nikolay, > > > >> > > > >> I've just closed them all. > > > >> > > > >> And thanks to you all for working to keep Kafka development > > > >> healthy! > > > >> > > > >> -John > > > >> > > > >> On Wed, 2022-02-09 at 14:19 +0300, Nikolay Izhikov wrote: > > > >>> Hello, guys. > > > >>> > > > >>> I made a quick search throw oldest PRs. > > > >>> Looks like the following list can be safely closed. > > > >>> > > > >>> Committers, can you, please, push the actual «close» button for > this > > > list of PRs? > > > >>> > > > >>> https://github.com/apache/kafka/pull/560 > > > >>> https://github.com/apache/kafka/pull/200 > > > >>> https://github.com/apache/kafka/pull/62 > > > >>> https://github.com/apache/kafka/pull/719 > > > >>> https://github.com/apache/kafka/pull/735 > > > >>> https://github.com/apache/kafka/pull/757 > > > >>> https://github.com/apache/kafka/pull/824 > > > >>> https://github.com/apache/kafka/pull/880 > > > >>> https://github.com/apache/kafka/pull/907 > > > >>> https://github.com/apache/kafka/pull/983 > > > >>> https://github.com/apache/kafka/pull/1035 > > > >>> https://github.com/apache/kafka/pull/1078 > > > >>> https://github.com/apache/kafka/pull/1111 > > > >>> https://github.com/apache/kafka/pull/1135 > > > >>> https://github.com/apache/kafka/pull/1147 > > > >>> https://github.com/apache/kafka/pull/1150 > > > >>> https://github.com/apache/kafka/pull/1244 > > > >>> https://github.com/apache/kafka/pull/1269 > > > >>> https://github.com/apache/kafka/pull/1415 > > > >>> https://github.com/apache/kafka/pull/1468 > > > >>> > > > >>>> 7 февр. 2022 г., в 20:04, Mickael Maison < > mickael.mai...@gmail.com> > > > написал(а): > > > >>>> > > > >>>> Hi David, > > > >>>> > > > >>>> I agree with you, I think we should close stale PRs. > > > >>>> > > > >>>> Overall, I think we should also see if there are other Github > > actions > > > >>>> that may ease the work for reviewers and/or give more visibility > of > > > >>>> the process to PR authors. > > > >>>> I'm thinking things like: > > > >>>> - code coverage changes > > > >>>> - better view on results from the build, for example if it's > failing > > > >>>> checkstyle, the author could be notified first > > > >>>> - check whether public API are touched and it requires a KIP > > > >>>> > > > >>>> For example, see some actions/integration used by other Apache > > > projects: > > > >>>> - Flink: > > > https://github.com/apache/flink/pull/18638#issuecomment-1030709579 > > > >>>> - Beam: > https://github.com/apache/beam/pull/16746#issue-1124656975 > > > >>>> - Pinot: > > > https://github.com/apache/pinot/pull/8139#issuecomment-1030701265 > > > >>>> > > > >>>> Finally, as several people have mentioned already, what can we do > to > > > >>>> increase the impact of contributors that are not (yet?) > committers? > > > >>>> Currently, our long delays in reviewing PRs and KIPs is hurting > the > > > >>>> project and we're for sure missing out some fixes and potential > > > >>>> contributors. I think Josep's idea is interesting and finding ways > > to > > > >>>> engage more people and share some responsibilities better will > > improve > > > >>>> the project. Currently the investment to become a committer is > > pretty > > > >>>> high. This could provide a stepping stone (or an intermediary > role) > > > >>>> for some people in the community. > > > >>>> > > > >>>> Thanks, > > > >>>> Mickael > > > >>>> > > > >>>> > > > >>>> On Mon, Feb 7, 2022 at 12:51 PM Josep Prat > > > <josep.p...@aiven.io.invalid> wrote: > > > >>>>> > > > >>>>> Hi, > > > >>>>> > > > >>>>> It seems like a great idea. I agree with you that we should use > > this > > > as a > > > >>>>> means to notify contributors and reviewers that there is some > work > > > to be > > > >>>>> done. > > > >>>>> > > > >>>>> Regarding labels, a couple of things, first one is that PR > > > participants > > > >>>>> won't get notified when a label is applied. So probably it would > be > > > best to > > > >>>>> apply a label and add a comment. > > > >>>>> Secondly, GitHub offers better fine-grained roles for > contributors: > > > read, > > > >>>>> triage, write, maintain, admin (further reading here: > > > >>>>> > > > > > > https://docs.github.com/en/organizations/managing-access-to-your-organizations-repositories/repository-roles-for-an-organization#permissions-for-each-role > > > ). > > > >>>>> One thing that might make sense to do maybe is to add frequent > > > contributors > > > >>>>> with the "triage" role, so they could label PRs they reviewed and > > > they can > > > >>>>> be taken by committers for a further review and potential merge. > > > What do > > > >>>>> you think? > > > >>>>> > > > >>>>> Best, > > > >>>>> > > > >>>>> On Mon, Feb 7, 2022 at 12:16 PM Nikolay Izhikov < > > nizhi...@apache.org> > > > wrote: > > > >>>>> > > > >>>>>>> We do not have a separate list of PRs that need pre-reviews. > > > >>>>>> > > > >>>>>> Ok. > > > >>>>>> What should I do if PR need to be to closed found? > > > >>>>>> Who can I tag to do actual close? > > > >>>>>> > > > >>>>>>> 7 февр. 2022 г., в 13:53, Bruno Cadonna <cado...@apache.org> > > > написал(а): > > > >>>>>>> > > > >>>>>>> Hi, > > > >>>>>>> > > > >>>>>>> Thank you David for bringing this up! > > > >>>>>>> > > > >>>>>>> I am in favour of automatically closing stale PRs. I agree with > > > Guozhang > > > >>>>>> that notifications of staleness to authors would be better than > > > silently > > > >>>>>> closing them. I assume the notification happens automatically > when > > > the > > > >>>>>> label "Stale" is added to the PR. > > > >>>>>>> > > > >>>>>>> +1 for Matthias' proposal of non-committers doing a pre-review. > > > That > > > >>>>>> would definitely save some time for committer reviews. > > > >>>>>>> > > > >>>>>>> Nikolay, great that you are willing to do reviews. We do not > > have a > > > >>>>>> separate list of PRs that need pre-reviews. You can consult the > > > list of PRs > > > >>>>>> of Apache Kafka (https://github.com/apache/kafka/pulls) and > > choose > > > from > > > >>>>>> there. I think that is the simplest way to start reviewing. > Maybe > > > Luke has > > > >>>>>> some tips here since he does an excellent job in reviewing as a > > > >>>>>> non-committer. > > > >>>>>>> > > > >>>>>>> Best, > > > >>>>>>> Bruno > > > >>>>>>> > > > >>>>>>> On 07.02.22 08:24, Nikolay Izhikov wrote: > > > >>>>>>>> Hello, Matthias, Luke. > > > >>>>>>>>> I agree with Matthias that contributors could and should help > > do > > > more > > > >>>>>> "pre-review" PRs. > > > >>>>>>>> I, personally, ready to do the initial review of PRs. Do we > have > > > some > > > >>>>>> recipe to filter PRs that has potential to land in trunk? > > > >>>>>>>> Can, you, please, send me list of PRs that need to be > > > pre-reviewed? > > > >>>>>>>>> I might be useful thought to just do a better job to update > KIP > > > status > > > >>>>>> more frequently > > > >>>>>>>> First, I thought it’s an author job to keep KIP status up to > > date. > > > >>>>>>>> But, it can be tricky to determine actual KIP status because > of > > > lack of > > > >>>>>> feedback from committers :) > > > >>>>>>>> Second - the other issue is determine - what KIP just wait > for a > > > hero > > > >>>>>> to implement it, and what just wrong idea or something similar. > > > >>>>>>>> All of this kind of KIPs has status «Under discussion». > > > >>>>>>>> Actually, if someone has a list of potentially useful KIPS - > > > please, > > > >>>>>> send it. > > > >>>>>>>> I’m ready to work on one of those. > > > >>>>>>>>> 7 февр. 2022 г., в 05:28, Luke Chen <show...@gmail.com> > > > написал(а): > > > >>>>>>>>> > > > >>>>>>>>> I agree with Matthias that contributors could and should help > > do > > > more > > > >>>>>>>>> "pre-review" PRs. > > > >>>>>>>>> Otherwise, we're not fixing the root cause of the issue, and > > > still > > > >>>>>> keeping > > > >>>>>>>>> piling up the PRs (and auto closing them after stale) > > > >>>>>>>>> > > > >>>>>>>>> And I also agree with Guozhang that we should try to notify > at > > > least > > > >>>>>> the > > > >>>>>>>>> committers about the closed PRs (maybe PR participants + > > > committers if > > > >>>>>>>>> possible). > > > >>>>>>>>> Although the PRs are stale, there might be some good PRs just > > got > > > >>>>>> ignored. > > > >>>>>>>>> > > > >>>>>>>>> Thank you. > > > >>>>>>>>> Luke > > > >>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>>> On Mon, Feb 7, 2022 at 6:50 AM Guozhang Wang < > > wangg...@gmail.com > > > > > > > >>>>>> wrote: > > > >>>>>>>>> > > > >>>>>>>>>> Thanks for bringing this up David. I'm in favor of some > > > automatic > > > >>>>>> ways to > > > >>>>>>>>>> clean up stale PRs. More specifically: > > > >>>>>>>>>> > > > >>>>>>>>>> * I think there are indeed many root causes why we have so > > many > > > stale > > > >>>>>> PRs > > > >>>>>>>>>> that we should consider, and admittedly the reviewing > manpower > > > cannot > > > >>>>>> keep > > > >>>>>>>>>> up with the contributing pace is a big one of them. But in > > this > > > >>>>>> discussion > > > >>>>>>>>>> I'd personally like to keep this out of the scope and maybe > > > keep it > > > >>>>>> as a > > > >>>>>>>>>> separate discussion (I think we are having some discussions > on > > > some of > > > >>>>>>>>>> these root causes in parallel at the moment). > > > >>>>>>>>>> > > > >>>>>>>>>> * As for just how to handle the existing stale PRs, I think > > > having an > > > >>>>>>>>>> automatic way would be possibly the most effective manner, > as > > I > > > >>>>>> suspect how > > > >>>>>>>>>> maintainable it would be to do that manually. The question > > > though > > > >>>>>> would be: > > > >>>>>>>>>> do we just automatically close those PRs silently or should > we > > > also > > > >>>>>> send > > > >>>>>>>>>> notifications along with it. It seems > > > >>>>>> https://github.com/actions/stale can > > > >>>>>>>>>> definitely do the first, but not sure if it could the > second? > > > Plus > > > >>>>>> let's > > > >>>>>>>>>> say if we want notifications and it's doable via Action, > could > > > we > > > >>>>>> configure > > > >>>>>>>>>> just the committers list (as sending notifications to all > > > community > > > >>>>>>>>>> subscribers may be too spammy)? Personally I feel setting 6 > > > months for > > > >>>>>>>>>> closing and notifying committers on a per-week basis seems > > > sufficient. > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> Guozhang > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> On Sun, Feb 6, 2022 at 9:58 AM Matthias J. Sax < > > > mj...@apache.org> > > > >>>>>> wrote: > > > >>>>>>>>>> > > > >>>>>>>>>>> I am +1 to close stale PRs -- not sure to what extend we > want > > > to > > > >>>>>>>>>>> automate it, or just leave it up to the committers to do > the > > > cleanup > > > >>>>>>>>>>> manually. I am happy both ways. > > > >>>>>>>>>>> > > > >>>>>>>>>>> However, I also want to point out, that one reason why we > > have > > > so > > > >>>>>> many > > > >>>>>>>>>>> stale PRs is the committer overload to actually review PRs. > > > It's a > > > >>>>>> pity > > > >>>>>>>>>>> that committer cannot keep up with the load (guilty as > > charged > > > >>>>>> myself). > > > >>>>>>>>>>> Not sure if it would help if more contributors could help > > doing > > > >>>>>> reviews, > > > >>>>>>>>>>> such that PRs are "pre-reviewed" and already in good shape > > > before a > > > >>>>>>>>>>> committer reviews it? > > > >>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>>> For KIPs, there is actually two more categories: > > > >>>>>>>>>>> > > > >>>>>>>>>>> - "Dormant/Inactive" > > > >>>>>>>>>>> - "Discarded: > > > >>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals#KafkaImprovementProposals-DiscardedKIPs > > > >>>>>>>>>>> > > > >>>>>>>>>>> For Kafka Streams in particular, we also try to make the > > > status of > > > >>>>>> KIP > > > >>>>>>>>>>> clear in the corresponding sub-page: > > > >>>>>>>>>>> > > > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Streams > > > >>>>>>>>>>> > > > >>>>>>>>>>> I might be useful thought to just do a better job to update > > KIP > > > >>>>>> status > > > >>>>>>>>>>> more frequently -- we could also re-organize the main KIP > > wiki > > > page > > > >>>>>> -- I > > > >>>>>>>>>>> think it contains too much information and is hard to read. > > > >>>>>>>>>>> > > > >>>>>>>>>>> For the Kafka Streams sub-page, we use it for all "active" > > > KIPs, > > > >>>>>> while > > > >>>>>>>>>>> we maintain a second page for adopted KIPs grouped by > > release: > > > >>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Streams+KIP+Overview > > > >>>>>>>>>>> > > > >>>>>>>>>>> I find this much more digestible compared to the main KIP > > page. > > > >>>>>>>>>>> > > > >>>>>>>>>>> Might also be good to have a sub-page for Connect KIPs? > > > >>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>>> -Matthias > > > >>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>>> On 2/5/22 05:57, Luke Chen wrote: > > > >>>>>>>>>>>> Hi Nikolay, > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> That's a good question! > > > >>>>>>>>>>>> But I think for stale KIP, we should have another > discussion > > > thread. > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> In my opinion, I agree we should also have similar > mechanism > > > for > > > >>>>>> KIP. > > > >>>>>>>>>>>> Currently, the state of KIP has "under discussion", > > "voting", > > > and > > > >>>>>>>>>>>> "accepted". > > > >>>>>>>>>>>> The KIP might stay in "discussion" or "voting" state > > forever. > > > >>>>>>>>>>>> We might be able to have a new state called "close" for > KIP. > > > >>>>>>>>>>>> And we can review those inactive KIPs for a long time like > > PR > > > did, > > > >>>>>> to > > > >>>>>>>>>> see > > > >>>>>>>>>>>> if these KIPs need to close or re-start the discussion > > again. > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> Thank you. > > > >>>>>>>>>>>> Luke > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> On Sat, Feb 5, 2022 at 9:23 PM Nikolay Izhikov < > > > nizhi...@apache.org > > > >>>>>>> > > > >>>>>>>>>>> wrote: > > > >>>>>>>>>>>> > > > >>>>>>>>>>>>> Hello, David, Luke. > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> What about KIPs? > > > >>>>>>>>>>>>> Should we have some special state on KIPs that was > rejected > > > or > > > >>>>>> can’t > > > >>>>>>>>>> be > > > >>>>>>>>>>>>> implemented due to lack of design or when Kafka goes in > > > another > > > >>>>>>>>>>> direction? > > > >>>>>>>>>>>>> Right now those kind of KIPs just have no feedback. > > > >>>>>>>>>>>>> For me as a contributor it’s not clear - what is wrong > with > > > the > > > >>>>>> KIP. > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> Is it wrong? Is there are no contributor to do the > > > implementation? > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>>> 5 февр. 2022 г., в 15:49, Luke Chen <show...@gmail.com> > > > >>>>>> написал(а): > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> Hi David, > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> I agree with it! This is also a good way to let both > > > parties (code > > > >>>>>>>>>>> author > > > >>>>>>>>>>>>>> and reviewers) know there's a PR is not active anymore. > > > Should we > > > >>>>>>>>>>>>> continue > > > >>>>>>>>>>>>>> it or close it directly? > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> In my opinion, 1 year is too long, half a year should be > > > long > > > >>>>>> enough. > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> Thank you. > > > >>>>>>>>>>>>>> Luke > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> On Sat, Feb 5, 2022 at 8:17 PM Sagar < > > > sagarmeansoc...@gmail.com> > > > >>>>>>>>>>> wrote: > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> Hey David, > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> That's a great idea.. Just to stress your point, this > > > keeps both > > > >>>>>>>>>>> parties > > > >>>>>>>>>>>>>>> informed if a PR has become stale. So, the reviewer > would > > > know > > > >>>>>> that > > > >>>>>>>>>>>>> there > > > >>>>>>>>>>>>>>> was some PR which was being reviewed but due to > > inactivity > > > it got > > > >>>>>>>>>>>>> closed so > > > >>>>>>>>>>>>>>> maybe time to relook and similarly the submitter. > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> And yeah, any stale/unused PRs can be closed straight > > away > > > >>>>>> thereby > > > >>>>>>>>>>>>> reducing > > > >>>>>>>>>>>>>>> the load on reviewers. I have done some work on > > kubernetes > > > open > > > >>>>>>>>>> source > > > >>>>>>>>>>>>> and > > > >>>>>>>>>>>>>>> they follow a similar paradigm which is useful. > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> Thanks! > > > >>>>>>>>>>>>>>> Sagar. > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> -- > > > >>>>>>>>>> -- Guozhang > > > >>>>>>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>> > > > >>>>> -- > > > >>>>> > > > >>>>> Josep Prat > > > >>>>> > > > >>>>> *Aiven Deutschland GmbH* > > > >>>>> > > > >>>>> Immanuelkirchstraße 26, 10405 Berlin > > > >>>>> > > > >>>>> Amtsgericht Charlottenburg, HRB 209739 B > > > >>>>> > > > >>>>> Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen > > > >>>>> > > > >>>>> *m:* +491715557497 > > > >>>>> > > > >>>>> *w:* aiven.io > > > >>>>> > > > >>>>> *e:* josep.p...@aiven.io > > > >>> > > > >> > > > > > > > > > > > > > -- > > -- Guozhang > > > -- -- Guozhang
