Gentle ping to surface this up for more discussions. Regards Venkata krishnan
On Tue, Sep 26, 2023 at 4:59 PM Venkatakrishnan Sowrirajan <vsowr...@asu.edu> wrote: > 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 >> > > >> >
