Should old PRs that look abandoned be closed?

Nixpkgs has the problem that a lot of contributions arrive but some of them don’t get the attention and pile up.

A lot of older contributions get lost in the pile, some of them get abandoned, so when to close them?

Right now we have contributions from 2017 that got no response [1]. Should these be closed as abandoned?

[1] Pull requests · NixOS/nixpkgs · GitHub

4 Likes

Yes but we should triage each of them one by one and determine if the PR is really stale or just missing review and if the author did not respond/work on it for a good amount of time (9 months? 12 months?).

3 Likes

I realize this is years old now, hopefully no one minds me reviving it. I was just thinking about this and wondered if there was a conversation started already.

I was imaging the heuristic:

  1. PR is older than 1 year
  2. PR has merge conflicts
  3. PR is not a draft (as drafts are more likely to sit for a while and then get revisited later)

Maybe better than (1) would be “PR has not had a comment or commit in over 1 year.” Also, (2) and (3) are very conservative, but I assume we would still make a little headway on closing stale PRs.

In my opinion this could be automated as long as there was a comment on the PR explaining the situation (something to effect of why the PR is being closed and welcoming the PR author to re-open). I don’t see this as likely to cause frustration for anyone (less so than the frustration they may have already felt over their PR sitting for so long to begin with). That’s just my opinion, though. It’s not a strong vote in this world since I am a very minor contributor to nixpkgs.

I like the idea as long as we don’t have to fight off a stalebot every few weeks.

A bot might be appealing to some, but I’d settle for a script that was manually run on rare occasions as a starting place.

I (personally) see an automated closure of PRs that fell through the cracks as a bit of a double-failure:

  1. it reinforces the perception that no one cared
  2. it misses a chance to leave breadcrumbs in the form of a summary or crosslink when relevant (i.e., superseded PR, duplicate issue, etc.), making the issue corpus as a whole less useful in the future

I don’t think that means automation can’t help, but I think it makes the most sense as an aid to human intervention?

For example, when I triage old PRs/issues my process tends to look something like:

  • Figure out if the issue/PR is still relevant (look for signs the issue has been fixed in the interim, rendered moot by the removal of the package, update/init landed in another PR, there’s already a superseding PR, etc.) and go ahead and close with a comment that offers to reopen if I’ve misunderstood. (I use saved replies to improve this process.)

    This process takes a bit of time, but it can lead to a number of obvious closures.

  • If I couldn’t tell via above, query the reporter/author about whether it’s still relevant and whether they are still interested in pursuing it. I’ll go ahead and ask for obvious changes like fixing merge conflicts.

    Just asking a question will lead to a fair number of self-closures.

    • If they don’t respond, close it the next time I see it. (This part could probably be automated with a label. Maybe we could re-use 9.needs: reporter feedback or ask for a new one. I’d love to be able to mark these for auto-closure in 30 days if the reporter/author doesn’t reply. (They still get the opportunity to interact with a person, but I’d retain leverage by not having to go manually review issues I’ve interacted with to see which ones are ripe to close.)
    • If it’s a PR and they are still interested, I fish around here or on Matrix for help getting them reviews. I may also advocate a bit on behalf of the author against shifting review goalposts if I think it’ll help land the PR. (i.e., ask reviewers not to make an author with a perfectly good PR that predated the by-name framework to refactor it to fit that framework)
  • If it did get reviews previously but seems to have bogged down around a lack of clarity/engagement from the reviewer, I may try to prod them into clarifying what they wanted.

Another place where I feel like automation might be able to help out here is in helping humans coordinate to avoid repeatedly triaging the same old issues independently. If we had something like a wellness-check queue and a way to both remove it from the queue and affirmatively mark that someone laid eyes on it, we could focus on spreading knowledge of that process among people who occasionally triage old reports.

I guess that could be as simple as automatically adding a label every 6-12 months and encouraging human reviewers to watch for reports with that label, remove the label or swap it for another as soon as they’ve taken one off the pile, etc.

3 Likes

What if we do it like a monthly cronjob that mentions the PRs in a automated issue?

First month the PR is marked for closure and if there is no activity like discussion or code the PR is closed.

That can serve as a heads up about PRs that may be into a limbo.

Older PRs could be pick up at random.

Like this month 10 PRs, for example, were opened and stayed in limbo. The automation would pick randomly 20% the amount, so 2 PRs from before implementation.

That would serve as a community wide headsup about stuck stuff.

Next month if those mentioned PRs had no activity, not a review, merge or sokething like that, then it would be closed automatically.

I think the biggest challenge there would be dealing with github ratelimits.

If they are old and open, what’s the problem? I’d say just leave them open, unless you’re writing a replacement PR, and the previous PR was pending contributor response…otherwise review the previous one and get it closer to merge.

1 Like

Looking at these PRs, I found this one which looks ready to merge but has been idle for years, and it’s probably not the only one
Do we have a topic akin to PRs ready for review for merging? I’ve noticed we have the needs merger and awaiting merger github tags, but they do not seem used in practice

See PRs already reviewed.

Yes, that was related to some automation from years ago, I think. The automation was disabled so the labels fell into disuse

1 Like