In the past we’ve discussed about closing stale PRs (prs with no activity, no comment or anything after a certain period), however we have never reached a decision on how many days of no activity that the PR can be considered stale.
There is now a GitHub Action for automatically markings PRs and issues as stale, and it can also automatically close the stale issue/PR.
It works as follows: if a the PR has no activity after certain (configurable) period, it will apply the “stale” label, and leave a comment to give a warning that this PR is now stale. If someone them leaves a comment in the PR (they can leave a comment saying “I’m waiting for review” or “still working on it” or anything) , then the stale label gets removed.
However if there continued to be no activity, no further comments after certain number of days, then the bot will close the PR.
Obviously I’m aware of how much time consuming it is do this job and it’s not a critique, but I’m afraid that if they are closed automatically, a lot of work would be lost.
Especially for new contributors that are still learning the process of contributing to Python, if they miss the message their PR will be closed and they need to track them carefully or their work will be forgotten which raises the bar for them.
I think the number of days before sending the message should be greater than the maximum time it can take for a core developer to get to a PR, maybe 6 months?
Yes it makes sense to me to have the “days before closing” to be really long like in months instead of days.
I think having the stale label will also be useful so coredevs and triagers can hopefully check the PRs that are stale periodically and prioritize reviewing such PRs.
Once marked as stale, does it go to the top of the PRs list because it has activity? Will it also go to the top of the list because the PR author will say “waiting for review”. That’s going to be more than 3 PRs brought to the top every day.
Is there any benefit to closing PRs? I can’t look at the code right now, so I’m not sure if it can be passed a null value for the days-until-close parameter
I’m not sure the distribution of age on our PRs. 60 days from stale to close seems reasonable if no activity. As for how long before stale (6 months?).
Just for some quick data, I ran some analysis on our pull requests. If anyone has any requests for any other graphs they’d be interested in or the raw data, feel free to reply here.
My two cents: we should lean a bit on the conservative side for the “time to mark as stale” and “time to close stale PRs”. Like @remilapeyre said, this can really dissuade contributors especially given how long reviews can take.
Based on the non core-dev data, given that 95% of pull requests are merged within 300 days, I would recommend that as the cut-off point for marking PRs as stale.
For time to close, setting it to 40-50 days seems reasonable since it looks like that amount of time is what it takes for authors to take actions on pull requests.
I do think it’s useful to close out really old PRs that haven’t been actioned on. It’s equally as annoying for reviewers when they go to review a patch that no longer merges cleanly. New contributors taking on a bug also have to wait for the stale PR author to respond back.
Here’s the quick and dirty code, it’s easy to generate the PR age distribution graph since you can retrieve 100 pull requests per request from Github (~215 requests in total currently). It’s harder to do ones based on labels since that needs a request per pull request to pull in all the data.
Code
from github import Github
import json
g = Github("GITHUB_TOKEN", per_page=100)
with open('pr_list.json', 'w', encoding='utf-8') as f:
repo = g.get_repo("python/cpython")
for i, pull in enumerate(repo.get_pulls('all')):
print("Retrieved {} pull requests".format(i))
f.write(json.dumps(pull._rawData))
f.write("\n")
Jupyter Notebook
pull_requests = {}
with open('pr_list.json', 'r', encoding='utf-8') as f:
for line in f:
raw_object = json.loads(line)
id = int(raw_object["url"].split("/")[-1])
pull_requests[id] = raw_object
merge_days = []
for pr in prs.values():
if pr['user'] in CORE_DEVS or pr['user'] in BOTS:
continue
time_to_merge = pendulum.parse(pr['merged_at']) - pendulum.parse(pr['created_at'])
merge_days.append(time_to_merge.in_days())
ax = sns.kdeplot(all_merge_times, cumulative=True, shade=True)
ax.set_xlim(left=0)
ax.set(xlabel='Days after PR Creation', ylabel='Pull requests merged % (cumulative)')
I think the more complex analysis can be done through the backups we take of the Github repo if we want to avoid hitting the github api for all the info.
I will repeat here that I am opposed to mindless (mechanical) closing of issues and PRs in general and especially those for IDLE. Earlier this year, I closed about 40 IDLE issues (as other than ‘fixed’) and any linked PRs. But I left some open. I am more or less working now on a few really old issues (as in 10 years).
There was a similar discussion in the matplotlib gitter, with both arguments that:
A large number of PRs can be discouraging for (new) contributors.
vs
Closing an inactive PR can also drive contributor away.
Both of which are good point, and I’m not going to delve into whether the PR is closed with/without manual/automatic warning or not. nor does this address whether the turnout time if fast or not and so whether the Number of PR is a good metric, but arguably that’s one of the most prominent (and default) things visible in a repo and finding out whether the turnaround time is short requires user active interaction with the repo which most people won’t do.
I personally don’t think there is a one answer fit all repository, still this have lead me to run this highly non-scientific poll:
Dear OSS contributors. Which feels the worst A) You started a PR on a project, have not finished/touched it in quite some time. PR get closed with a message “This looks inactive, feel free to reopen”. B) You want to contribute to project X, there are already 200+ opened PRs.
In general both groups have about 2.5 to 3 time more respondent feeling that coming across a repo with more than 200 PR is worse than having a PR closed for inactivity.
I’ll leave the analysis to the reader. Some of the tweets reply also have good points.
I will also point out that closing a PR for inactivity can be discouraging for O(1) user per closed PR, (basically anyone interacting with this PR in particular or coming across it, and realising it was closed)
while having a discouragingly high number of PRs will effect about anyone coming across the repo even before contributing. So my guess is that there are many people (at least N=1, me) that won’t even send a PR if this number is too high.
I also don’t talk about maintainer feeling of having too many PRs, personally I feel stressed, and have difficulty finding what to focus on when the number of PRs on repo I maintain is high.
Thanks for reading my rambling, and everyone for you consideration on this issue.
I have just closed a 3 year old PR about this issue. I think has stalled for too long
It sounds like folks here don’t mind the “stale” label but some folks don’t want the PR to be automatically closed. I have adjusted my PR so that the “stale” label will get added, but it doesn’t ever close the PR, by setting the days-before-closing value to -1.
Ultimately, and after quite a lot of internal discussion, we have instead favoured trying to surface the most in demand issues through an upvote approach, see the Top Issues Dashboard from the github-readme-stats repo, as an example.
We are still very much experimenting with how to fundamentally address the issue of #issues opened vs contributor bandwidth to fix. Has your (et al) thinking evolved since this thread emerged, and did you ever look at encouraging prioritisation vs. force closing of stale issues?
I’m aware this thread is quite old now, apologies for bumping it to prominance again. However, any and all feedback as to the above topic, particularly learnings from implementing your own stale/close strategy, would be most interesting to hear.
For CPython, we only label issues as stale, and don’t do anything else like automatically closing issues.
However, it’s not running as expected right now because we have ~7,000 issues and the action only processes a subset each time, and saves a small cache to keep track of its progress. But we generate a lot of caches for the regular CI builds, which means we exceed the cache quota, the stale action’s cache is deleted and so it checks the same initial set every time. (See python/cpython#113858 for a tracking issue on the caches.)
For Pillow, we’re use the stale action for just one thing: when we ask a reporter for more information, but think they might not reply, we add a label, and if they don’t reply in a week, the issue is auto-closed. It’s woking well.
Yeah, I set up something like that for Spyder years and years ago (before GitHub Actions existed, so it was only semi-automated, though IIRC we’ve automated it since) with a awaiting response tag, and would close after 8 days. That helped greatly cut down on stale issues, since unfortunately many if not most issues did not provide enough information to clearly reproduce, and many reporters never responded. I’m not sure how much this would help for CPython since this seems to be less of a problem, but it certainly wouldn’t hurt.
I don’t feel that stale issues are a big problem for CPython – nobody can keep all the issues in their head and we just live with it. For issues that get my attention (mostly asyncio) if I need more info I set the pending label and if I get no response I may eventually close it – but a pending issue doesn’t cost me personally any more anguish than a closed issue. I guess for people with a “completionist” mindset it may be different.
Hugo’s cron job that occasionally tries to auto-label batches of issues should probably just use a more persistent form of storage for its state instead of a cache that may be evicted due to unrelated activity.
