Auto closing old stale PRs

[mhdawson]

Our backlog of feature requests has gone down from 249 to 87 with the stale feature request automation we put in place. A number of older ones are still open and we did not get too many complaints so I think it was easy enough for people to keep feature requests open that were still active/needed etc. while at the same time closing those that would likely never get addressed.

Looking at our PRs we have 340 open, 117 which are older than 1 year (https://github.com/nodejs/node/pulls?q=is%3Apr+is%3Aopen+created%3A%3C2021-05-05+).

When I look at those, in many cases the last comment is also a long time ago.

My thought is that for a PR 1 year old, which does not have any comments/updates in the last 6 months it's very unlikely that it's ever going to land. The longer it goes, the more likelyhood of conflicts etc. and that the original poster will no longer be around to help get it over the finish line.

Even in some cases where the PR looks like it was ready to land, we need something to kick it to our attention, or to get the originator to remind us its ready to land if they still care.

It's strange that 106 of those old PRs show as updated on Feb 24th, even though I can't see how they were updated when I look at the issues. Ignoring this issue I think it would be resonable to use an approach similar to what we did for feature requests with a different set of criteria.

PRs older than 1 year, no comments in last 5 months -> warn that PR is considered stale and will be closed if there is no comment in the next month
PRs older than 1 year, no comments in last 6 months -> close with appropriate message.

Thoughts?

[ljharb]

My experience is that non-humans closing things comes across as hostile (lacking compassion, etc). Using a bot to auto-tag stale issues with a human manually closing them later is fine; using a bot to auto-close them is a problem.

[mhdawson]

@ljharb I understand where you are coming from but I think the problem is that a human manually closing them later does not occur. I see issues where people periodically comment "is this still active" a number of times which streches over months or possibly longer. In the node-addon-api repo I've seen the stale autotagging and knowing that they will be closed without action has helped raised the visibility of stale issues/PRs and people being proactive because they want to avoid the autoclose when it does not make sense.

[ljharb]

@mhdawson in my experience it causes a bunch of much noisier "bump" comments, or, it gets closed (and in some projects, auto-locked) and then the problem is forgotten, which is much worse than the open issue being forgotten.

There's no value in optimizing for "fewer open issues" - the goal should be to optimize for fewer bugs.

[mhdawson]

@ljharb I see it as optimizing it for "more value added work" versus "fewer open issues". In this case we are talking only about PR's (I excluded issues as the dynamics are different there). I think a 1 year + stale PR only wastes peoples time (if they go look at PRs and try to help review/land) versus adding value at some point.

[ljharb]

I certainly agree that confining it to just PRs, and having the stale period be a full year (with a warning comment and a waiting period after that), drastically reduces my concerns (but doesn't eliminate them)

[mcollina]

I'm +1 with @mhdawson proposal.

[fhinkel]

+1

I think a bot auto-closing is friendlier than a human closing it.

If we have hundreds of open pull requests, they're equally "forgotten" as if we close them.

[bnoordhuis]

Also +1. I was going through some old PRs this morning that have 0% chance of getting merged but I didn't feel like closing them because you just know it's going to turn into a tedious back-and-forth when the author disagrees. Better to have a bot do it.

edit: I agree however with @ljharb about "bump" comments. I would make the bot strongly discourage them.

[BridgeAR]

Also +1 to the suggestion.

[mhdawson]

Discussed in the last TSC meeting no objection to proceed.

[gireeshpunathil]

I agree to the motivation - reducing the backlog and keeping it small and healthy. On the other hand, there are class of PRs (and the human behind those PRs) that can get hurt a little:

• PRs that are pending on collaborators
• PRs that have objections from reviewers with no path forward suggested
• PRs that were carried away (transformed) in the review and went out of OP's control and skill
• PRs that reveals flakes in the CI
• PRs whose OP is genuinely busy with other matters

I am not saying that all the PRs belong to these categories, or all these types can cause pains to the owners. My personal opinion is that we should make our PR abandonment process as friendly as possible (complimenting with the onboarding process) - a human touch would be much comforting for the owner.

Can I propose TSC members tasked with reviewing stale PRs each week (in small numbers) and address this one in an incremental, yet sustainable and firendly manner?

[mhdawson]

Removed from the agenda as I think we agreed we could have the renamining discussion in the PR to add the action which will have the details of when/what might be closed.

[ljharb]

It's unfortunate that there wasn't any ping here that a PR had been opened, so we could actually have the remaining discussion there :-/

[mhdawson]

Unfortunately all of our PRs seemed to have been updated for some unknown reason on May 1st of this year (so far I can't see why in the Github UI but both the action and the queries in the UI seem to agree they were updated).

The last update time seems to be that date or later on most if not all issues.

Unless we want to change the criteria to be other than not updated in 6 months that means we'll have to wait until October 1 to start having the PRs be tagged.

Reopening this so that we revisit when we hit that date.

I'd also be open to shortening the time to not updated in the last month for those that are 2 more more years old. We currently have 105 - https://github.com/nodejs/node/pulls?q=is%3Apr+is%3Aopen+created%3A%3C2021-06-05+

I would be willing to have the bot run on those at about 20 per week but running the bot manually once a week with a date so that we get ~20. I looked at about 5-6 and none of them seemed to have something I'd see as an update in over 6 moths.

[BethGriggs]

Unfortunately all of our PRs seemed to have been updated for some unknown reason on May 1st of this year (so far I can't see why in the Github UI but both the action and the queries in the UI seem to agree they were updated).

Looking at #nodejs-core Slack the last force-push of main was on May 1st, which could be the cause? If that is the case then the approach may need to be adjusted (as we'll probably happen to force-push main again within the next 6 months).

[mhdawson]

That could explain it. The issues showed a force push 7 months ago but not that one (unless I just missed that somehow). Otherwise I would have been looking for something like that.

The problem is that a force push will affect all of our use of the stale actions (not just this new one) if they occur within every 6 months.