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

Reply via email to