Pytest: Proposal on how to deal with stale PRs

[nicoddemus]

Hi everyone,

Currently we have a bunch of PRs open (50+), and I notice many of them are "stale", meaning that we have asked for changes and the authors didn't got around to implement them yet after a somewhat long time.

There are many reasons why people don't implement changes: they might get busy, lost interest in a particular issue, or plainly just forgot about it.

In the past we used to send "gentle pings" to authors, but this does not scale very well: we still need to go back to the large list of open PRs periodically to identify stale ones where we have already sent a ping to users, before deciding to close it because of lack of response. And this is a bit tiring, because often you need to re-read a whole thread to identify if a PR is stale or waiting for an action on our part.

So I would like to reduce the large number of open PRs, because having this large a list makes it easy to lose some notifications about changes.

So in order to do that, I propose: from now on we close PRs which we have identified as stale right away, leaving a nice message to the user explaining that we really appreciate their efforts, but we want to close the PRs to clean off our queue, and encouraging them to re-open the PR once they get back to it. This is just to be meant as a gentle reminder, because users can easily re-open an existing PR at the click of a button. Of course we need to write a nicely worded message so users, which took time and effort to contribute, don't feel offended or brushed off.

Just to make sure I get my point across, here's an example of a message I think would be appropriate while closing a stale PR:

---

Hi ,

First of all we would like to thank you for your time and effort on working on this, the pytest team deeply appreciates it.

We noticed it has been awhile since you have updated this PR, however. pytest is a high activity project, with may issues/PRs being opened daily, so it is hard for us maintainers to track which PRs are ready for merging, for review, or need more attention.

So for those reasons we think it is best to close the PR for now, but with the only intention to cleanup our queue, it is by no means a rejection of your changes. We still encourage you to re-open this PR (it is just a click of a button away) when you are ready to get back to it.

Again we appreciate your time for working on this, and hope you might get back to this at a later time!

Thanks, the pytest team.

---

Thoughts?

[RonnyPfannschmidt]

I like the idea, it would be nice to have a bot that manages staleness, pings and closes

[nicoddemus]

There's https://github.com/probot/stale, which has all configurations we need: customize number of days for ping, closing, and limiting only to PRs.

[gnikonorov]

Any chance this can be added to issues too? I see that it has support for issues, and speaking as someone that is brand new to trying to contributing to pytest, the large issue list is very confusing as it would appear that many may be abandoned/out of date.

[nicoddemus]

Hi @gnikonorov,

Any chance this can be added to issues too?

I'm not sure, there's a lot of issues in the tracker that are still relevant and should be worked on eventually. But I agree it is hard to navigate the tracker and identify what's relevant still or not.

[gnikonorov]

@nicoddemus I see prbot has the config option exemptLabels which makes it ignore anything with that label. Would it be too much to ask issue responders/issue submitters to mark the issue with a label in exemptLabels if it is a genuine bug? We can also configure prbot to ignore issues with an assignee.

I know this would be a headache for existing issues, but on the plus side it would help clean up the issue list.

Just a thought, I know this is a big ask time commitment wise from the core development team

[DahlitzFlorian]

I like the idea. However, there is another point one should talk about:
There are PRs, which are nearly finished, so only a few changes need to be made, but the author does not get to it/respond or whatever. Would it be an option to finish these PRs and merge them if the effort is not that huge?

The benefit would be that the work is not lost and the original author is credited, too.

[symonk]

Good idea +1. Would also like to see more marking of ‘easy’ issues as that’s where people tend to start -maybe there haven’t been that many recently but the marker seems somewhat neglected

[nicoddemus]

There are PRs, which are nearly finished, so only a few changes need to be made, but the author does not get to it/respond or whatever. Would it be an option to finish these PRs and merge them if the effort is not that huge?

Oh definitely, thanks for bringing that up.

Would also like to see more marking of ‘easy’ issues as that’s where people tend to start -maybe there haven’t been that many recently but the marker seems somewhat neglected

You are right, sometimes it seems we miss applying that label. 👍

[asottile]

I do agree that we should close stale things, but I'd rather see us do it as humans and not use stalebot. personally I find stalebot to be kind of a rude way to handle issues (tedious for users to ping-open their tickets, really annoying if you turn on locking as well).

[nicoddemus]

but I'd rather see us do it as humans and not use stalebot.

Are you talking about issues, PRs or both?

For PRs, I'm OK with that too, because it is easy to filter by "update date" (say last updated 30 days ago) and close them with a template message.

But for issues that does not scale unfortunately.

[asottile]

yeah I meant more for PRs -- for issues you're right it's pretty difficult to stay on top of them.

for bugs I think we should validate that they're still issues (maybe automatically though some metadata in the issue itself?) and keep them open. for feature requests I'm fine closing them if they're stale.

[nicoddemus]

By the way, my main concern right now are stale PRs, so if we want to diverge the discussion on how to handle stale issues, I would rather open another topic. 😁

[asottile]

sure! let's split the issue discussion off from this

[nicoddemus]

So, I propose we do this process manually then, given @asottile's reservations.

Should we add this decision/process somewhere? https://docs.pytest.org/en/latest/contributing.html might be a fit, given it has sections about issues, backporting, etc. If not, any other suggestions?

[Zac-HD]

I'd be happy to add stale issues / PRs to the issue tagging that I go through reguarly, so long as we document somewhere what the process is. For example:

• issues tagged question - closed after 14 days inactive (status quo!)
• issues tagged proposal - closed after six months inactive (new rule)
• pull requests checked after one month inactive; update linked issue with lessons, ping author, consider closing

etc.

[symonk]

thoughts on also adding some documentation into the repo on how we classify issue(s) ? @Zac-HD seems to be on the ball with this one :D I think i recall reading the pytest .pdf docs which outlined it in depth, perhaps a issue_guide.rst/.md could be useful for someone new?

[RonnyPfannschmidt]

i would like us to have some better way to sort that, currently i observe that @Zac-HD on rare occasions closing issues that i reopen afterwards due to having more knowledge of the context (while i appreciate the progressive approach to closing)

[nicoddemus]

I agree, so where should we document the closing process? In contributing, like I mentioned before, or somewhere else?

[RonnyPfannschmidt]

i think thats a good starting point, we might want to consolidate all those contribution documentations and standardize across the pytest-dev org as a followup

[nicoddemus]

Sounds good.

@Zac-HD would you like to tackle this?

[Zac-HD]

All my suggestions are already in the thread! I was actually hoping that someone else would write it up, because I've really just been guessing based on the existing labels and only mostly getting it right :smile: