Hi Martijn,

Agree with your point that closing a PR without any review feedback even
after 'X' days is discouraging to a new contributor. I understand that this
is a capacity problem. Capacity problem cannot be solved by this proposal
and it is beyond the scope of this proposal.

Regarding your earlier question,
> What's the added value of
closing these PRs

   - Having lots of inactive PRs lingering around shows the project is less
   active. I am not saying this is the only way to determine how active a
   project is, but this is one of the key factors.
   - A large number of PRs open can be discouraging for (new) contributors
   but on the other hand I agree closing an inactive PR without any reviews
   can also drive contributors away.

Having said all of that, I agree closing PRs that don't have any reviews to
start with should be avoided from the final proposal.

> I'm +1 for (automatically) closing up PRs after X days which:
a) Don't have a CI that has passed
b) Don't follow the code contribution guide (like commit naming conventions)
c) Have changes requested but aren't being followed-up by the contributor

In general, I'm largely +1 on your above proposal except for the
implementation feasibility.

Also, I have picked a few other popular projects that have implemented the
Github's actions stale rule to see if we can borrow some ideas. Below
projects are listed in the order of the most invasive (for lack of a better
word) to the least invasive actions taken wrt PR without any updates for a
long period of time.

1. Trino

TL;DR - No updates in the PR for the last 21 days, tag other maintainers
for review. If there are no updates for 21 days after that, close the PR
with this message - "*Closing this pull request, as it has been stale for
six weeks. Feel free to re-open at any time.*"
Trino's stale PR Github action rule (stale.yaml)
<https://github.com/trinodb/trino/blob/master/.github/workflows/stale.yml>

2. Apache Spark

TL;DR - No updates in the PR in the last 100 days, closing the PR with this
message - "*We're closing this PR because it hasn't been updated in a
while. This isn't a judgement on the merit of the PR in any way. It's
just a way of keeping the PR queue manageable. If you'd like to revive this
PR, please reopen it and ask a committer to remove the Stale tag!*"
Spark's discussion in their mailing list
<https://lists.apache.org/thread/yg3ggtvpt2dbjpnb2q0yblq30sc1g2yx> on
closing stale PRs. Spark's stale PR github action rule (stale.yaml
<https://github.com/apache/spark/blob/master/.github/workflows/stale.yml>).

3. Python

TL;DR - No updates in the PR for the last 30 days, then tag the PR as
stale. Note: Python project *doesn't* close the stale PRs.

Python discussion
<https://discuss.python.org/t/decision-needed-should-we-close-stale-prs-and-how-many-lapsed-days-are-prs-considered-stale/4637>
in the mailing list to close stale PRs. Python's stale PR github action
rule (stale.yaml
<https://github.com/python/cpython/blob/main/.github/workflows/stale.yml>)

Few others Apache Beam
<https://github.com/apache/beam/blob/master/.github/workflows/stale.yml>
(closes
inactive PRs after 60+ days), Apache Airflow
<https://github.com/apache/airflow/blob/main/.github/workflows/stale.yml>
(closes
inactive PRs after 50 days)

Let me know what you think. Looking forward to hearing from others in the
community and their experiences.

[1] Github Action - Close Stale Issues -
https://github.com/marketplace/actions/close-stale-issues

Regards
Venkata krishnan


On Thu, Sep 21, 2023 at 6:03 AM Martijn Visser <martijnvis...@apache.org>
wrote:

> Hi all,
>
> I really believe that the problem of the number of open PRs is just
> that there aren't enough reviewers/resources available to review them.
>
> > Stale PRs can clutter the repository, and closing them helps keep it
> organized and ensures that only relevant and up-to-date PRs are present.
>
> Sure, but what's the indicator that the PR is stale? The fact that
> there has been no reviewer yet to review it, doesn't mean that the PR
> is stale. For me, a stale PR is a PR that has been reviewed, changes
> have been requested and the contributor isn't participating in the
> discussion anymore. But that's a different story compared to closing
> PRs where there has been no review done at all.
>
> > It mainly helps the project maintainers/reviewers to focus on only the
> actively updated trimmed list of PRs that are ready for review.
>
> I disagree that closing PRs helps with this. If you want to help
> maintainers/reviewers, we should have a situation where it's obvious
> that a PR is really ready (meaning, CI has passed, PR contents/commit
> message etc are following the code contribution guidelines).
>
> > It helps Flink users who are waiting on a PR that enhances an existing
> feature or fixes an issue a clear indication on whether the PR will be
> continually worked on and eventually get a closure or not and therefore
> will be closed.
>
> Having other PRs being closed doesn't increase the guarantee that
> other PRs will be reviewed. It's still a capacity problem.
>
> > It would be demotivating for any contributor when there is no feedback
> for a PR within a sufficient period of time anyway.
>
> Definitely. But I think it would be even worse if someone makes a
> contribution, there is no response but after X days they get a message
> that their PR was closed automatically.
>
> I'm +1 for (automatically) closing up PRs after X days which:
> a) Don't have a CI that has passed
> b) Don't follow the code contribution guide (like commit naming
> conventions)
> c) Have changes requested but aren't being followed-up by the contributor
>
> I'm -1 for automatically closing PRs where no maintainers have taken a
> review for the reasons I've listed above.
>
> Best regards,
>
> Martijn
>
> On Wed, Sep 20, 2023 at 7:41 AM Venkatakrishnan Sowrirajan
> <vsowr...@asu.edu> wrote:
> >
> > Thanks for your response, Martijn.
> >
> > > What's the added value of
> > closing these PRs
> >
> > It mainly helps the project maintainers/reviewers to focus on only the
> > actively updated trimmed list of PRs that are ready for review.
> >
> > It helps Flink users who are waiting on a PR that enhances an existing
> > feature or fixes an issue a clear indication on whether the PR will be
> > continually worked on and eventually get a closure or not and therefore
> > will be closed.
> >
> > Btw, I am open to other suggestions or enhancements on top of the
> proposal
> > as well.
> >
> > > it would
> > just close PRs where maintainers haven't been able to perform a
> > review, but getting a PR closed without any feedback is also
> > demotivating for a (potential new) contributor
> >
> > It would be demotivating for any contributor when there is no feedback
> for
> > a PR within a sufficient period of time anyway. I don't see closing the
> PR
> > which is inactive after a sufficient period of time (say 60 to 90 days)
> > would be any more discouraging than not getting any feedback. The problem
> > of not getting feedback due to not enough maintainer's bandwidth has to
> be
> > solved through other mechanisms.
> >
> > > I think the important
> > thing is that we get into a cycle where maintainers can see which PRs
> > are ready for review, and also a way to divide the bulk of the work.
> >
> > Yes, exactly my point as well. It helps the maintainers to see a trimmed
> > list which is ready to be reviewed.
> >
> > +1 for the other automation to nudge/help the contributor to fix the PR
> > that follows the contribution guide, CI checks passed etc.
> >
> > > IIRC we can't really fix that until we can
> > finally move to dedicated Github Action Runners instead of the current
> > setup with Azure, but that's primarily blocked by ASF Infra.
> >
> > Curious, if you can share the JIRA or prior discussion on this topic. I
> > would like to learn more about why Github Actions cannot be used for
> Apache
> > Flink.
> >
> > Regards
> > Venkata krishnan
> >
> >
> > On Tue, Sep 19, 2023 at 2:00 PM Martijn Visser <martijnvis...@apache.org
> >
> > wrote:
> >
> > > Hi Venkata,
> > >
> > > Thanks for opening the discussion, I've been thinking about it quite a
> > > bit but I'm not sure what's the right approach.
> > >
> > > From your proposal, the question would be "What's the added value of
> > > closing these PRs"? I don't see an immediate value of that: it would
> > > just close PRs where maintainers haven't been able to perform a
> > > review, but getting a PR closed without any feedback is also
> > > demotivating for a (potential new) contributor. I think the important
> > > thing is that we get into a cycle where maintainers can see which PRs
> > > are ready for review, and also a way to divide the bulk of the work.
> > > Because doing proper reviews requires time, and these resources are
> > > scarce.
> > >
> > > I do think that we can make lives a bit easier with some automation:
> > > * There are a lot of PRs which don't follow the contribution guide (No
> > > Jira ticket, no correct commit message etc). For the externalized
> > > connector repositories, we've been trying Boring Cyborg to provide
> > > information back to contributors if their PRs are as expected. If the
> > > PR doesn't follow the contribution guide, I'm included to give such a
> > > PR less attention review. That's primarily because there are other PRs
> > > out there that do follow these guides.
> > > * There are even more PRs where the CI has failed: in those cases, a
> > > review also makes less sense, given that the PR can't be merged as is.
> > > I do see that contributors sometimes don't know where to look for the
> > > status of the CI, but IIRC we can't really fix that until we can
> > > finally move to dedicated Github Action Runners instead of the current
> > > setup with Azure, but that's primarily blocked by ASF Infra.
> > >
> > > I'm curious what others in the community think.
> > >
> > > Best regards,
> > >
> > > Martijn
> > >
> > > On Tue, Sep 19, 2023 at 10:33 PM Venkatakrishnan Sowrirajan
> > > <vsowr...@asu.edu> wrote:
> > > >
> > > > Hi Flink devs,
> > > >
> > > > There are currently over 1,000 open pull requests
> > > > <
> > >
> https://urldefense.com/v3/__https://github.com/apache/flink/pulls?q=is*3Aopen*is*3Apr*sort*3Aupdated-asc__;JSslKyU!!IKRxdwAv5BmarQ!eG42_TepSvUmlfJU0BqDRdhjGzm0eyim7YuyxEuawv0TakQL8aCI3EkbRc0ktXoGbZxFQsyB4uHYVM2yfzhRnomS$
> > > >
> > > > (PRs) in the Apache Flink repository, with only 162 having been
> updated
> > > in
> > > > the last two months
> > > > <
> > >
> https://urldefense.com/v3/__https://github.com/apache/flink/pulls?q=is*3Aopen*is*3Apr*sort*3Aupdated-asc*updated*3A*3E2023-07-19__;JSslKyUrJSU!!IKRxdwAv5BmarQ!eG42_TepSvUmlfJU0BqDRdhjGzm0eyim7YuyxEuawv0TakQL8aCI3EkbRc0ktXoGbZxFQsyB4uHYVM2yf4kpxy62$
> > > >.
> > > > This means that more than 85% of the PRs are stale and have not been
> > > > touched.
> > > >
> > > > I suggest setting up Github actions to monitor these stale PRs, and
> > > > automatically closing them if they have not been updated in the last
> 'x'
> > > > days. Authors would still be able to reopen the closed PRs if they
> > > continue
> > > > with their work. This would help to keep the PR queue manageable.
> > > >
> > > > Not sure if this has been discussed in the Apache Flink community
> before.
> > > >
> > > > Thoughts?
> > > >
> > > > Regards
> > > > Venkata krishnan
> > >
>

Reply via email to