Sounds good from our point of view as long as the time frame is reasonably far in the future (years rather than weeks).

This reeks of papering over the problem (which is that nobody is willing to code-review stuff) and pretending it's not a problem rather than actually solving it.

Actually, looking at this more closely I could support this if changeCleanup.abandonIfMergeable is set to false, otherwise I really don't want the experience that nobody reviews your patch and then some automated system declares it dead.

FWIW, https://gerrit.wikimedia.org/r/c/mediawiki/core/+/637656 sticks in my mind as an example of a change that ended up being merged quite a while after it was originally proposed (in this case, ~4 years afterwards). It's not immediately clear to me whether it gained any merge conflicts in that time.

See also T113706: Define Cut-off date for abandoning old changesets in our code review tool

I've been wanting this for years. As a maintainer of over 30+ codebases, stale patchsets disrupt my ability to get code review to people that need it (e.g. are still waiting for us to respond/engage with them or need help). I highly suspect that automatically abandoning older patchsets will actually lead to happier volunteers, more code reviews and more patches landing if we were to actually enforce this and measure impact.

I would expect any automation to:

1. Use updated time rather than created. If a patch was created 5 years ago but the author has been keeping it rebased since then regularly, it shouldn't be abandoned.
2. It should be possible to stop the automation by commenting on the patchset after a warning has gone out or making some kind of update to it.
3. It might make sense as a first pass to apply the critera of only abandoning patches with merge conflicts (since typically they often require a complete rewrite anyway to be reviewable)

FWIW speaking as one of the top +2ers in Gerrit, comments on patchsets (even if from a bot warning me it's going to be abandoned) is likely a desirable impetus for me that will lead to more patches getting merging. Since I have so many codebases I monitor I often forget about tickets, so appreciate pings of any kind. Particularly in MediaWiki core: it's really hard currently to detect patches that are in need of review.

If I understand correctly, we're proposing reducing the number of open patchsets, not drive by a benefit to patch writers or code reviewers, but by to the "Merge conflict detected" feature in Gerrit having trouble keeping up.

How useful is this feature? Who is using this? And, if we wanted to, is there a way to turn it off?

The merge-conflict indicator is not particulary prominent, so I imagine most people have never noticed it, or don't know what it means. Personally, I find that when I do notice it, I have never once acted on it. The information also is often inaccurate, reporting conflicts in patches that merge fine.

Relatedly, we do actually have another system already that also reports merge conflicts, and this one doesn't have the scaling issues of this Gerrit feature. I am referring to CI. When Jenkins/Zuul runs, it naturally has to perform a local git-merge before running the test. Any patches with merge conflicts are thus already reported as such by jenkins-bot. In addition there is also the Rebase button in Gerrit. It is common to rebase an old patch before reviewing it, both because Gerrit will then tell you if it has a merge conflict, and because it is helpful know if CI still passes before spending time on an old patch, e.g. to highlight any linting, structure, or integration issues with the latest code.

In T393376#10831580, @Krinkle wrote:

How useful is this feature? Who is using this? And, if we wanted to, is there a way to turn it off?

Yes. change.mergeabilityComputationBehavior = Never would do it. The default is to keep it disabled since it's very slow. The main feature I'd miss is the search for is:mergeable, but I don't use it often. It would be ideal to be able to have this feature only calculate for changes newer than X: that would achieve the same thing as closing old changes. That is not an option currently, though. There's an open feature request to have this on a per project basis. Likewise, I'm unsure if scap backport depends on this, but looking through the code, it seems like scap does look at mergeability, but doesn't trust gerrit's answers.

The merge-conflict indicator is not particulary prominent, so I imagine most people have never noticed it, or don't know what it means. Personally, I find that when I do notice it, I have never once acted on it. The information also is often inaccurate, reporting conflicts in patches that merge fine.

False positives are likely due to merge settings for a repo. If it's set to "merge if necessary" then a fancier merge could work vs. a fast-forward only. Mergeability could also be out of date due to being slow to calculate/update.

In T393376#10831580, @Krinkle wrote:

If I understand correctly, we're proposing reducing the number of open patchsets, not drive by a benefit to patch writers or code reviewers, but by to the "Merge conflict detected" feature in Gerrit having trouble keeping up.

Good news: someone wrote their doctoral thesis on the impact of using a stale bot (e.g., github actions stale).

Bad news: it's 160 pages.

But I skimmed it.

Here's the gist (emphasis mine):

We find that the studied projects closed more PRs within the first few months of adopting Stale bot, but overall closed and merged fewer PRs afterward. The adoption of Stale bot is also associated with faster first reviews in merged PRs, faster resolutions in closed PRs, slightly fewer updates in merged PRs, and considerably fewer active contributors in the projects.

– Sayed Hassan Khatoonabadi, (2023) Pull Request Abandonment in Open-Source Projects. PhD thesis

That last part is a little scary. The direct quote is: "overall 14% fewer contributors were active each month during the first year of adoption"

Caveats: the thesis only looked at 20 GitHub projects, but some there are some big ones (nixos, homeassistant, homebrew, grafana) and some are pretty comparable: wikia/app for example.

And while we know intuitively know that old, unreviewed changes are demotivating for contributors, at least one study of >1M pull requests indicates that first responses have middling correlation with future interactions.

tl;dr on research: stale pull requests are bad, but better than auto-abandoning them.

See also the approach in T200324: Create list of extensions whose Gerrit patches have not received reviews by the corresponding +2 Gerrit group (=maintainers) in a certain timeframe to identify likely dead projects.

Please don't automatically abandon changes just because they are old. I've had that happen when trying to contribute to other open source projects (I've also had my bug reports closed just because they were old) and it was very discouraging.

What would make sense to me, though, would be to abandon changes that have old unresolved CR-1 votes. If someone hasn't responded to feedback from a human reviewer in a couple of months or a year, they're probably not going to come back to it. I'm not sure if this should be automated or just a policy with the last step being someone clicking a button, but it'd probably be acceptable either way.

And on the other topic: I use the mergeable status often when looking through my dashboard, mostly when looking at my own changes (to know if I need to rebase them), but also sometimes to ping other people when a change they proposed (and that I want to approve) is not mergeable. I'd be sad if that was gone.

Please don't automatically abandon changes just because they are old. I've had that happen when trying to contribute to other open source projects (I've also had my bug reports closed just because they were old) and it was very discouraging.

+100 !