DOC: Add a policy about inactive PRs.

[InessaPawson]

Adding a policy about inactive PRs that was approved at the community meeting on March 30th, 2022. (Please refer to the meeting notes: https://github.com/numpy/archive/blob/3cd5b6ce3af12b649c3b053265e060393a25c26f/community_meetings/community-2022-03-30.md.)

[rossbar]

Formalizing a policy here seems fine to me - my one comment is that I'd vote to drop the "quorum of at least two maintainers" part as this is more restrictive than what is typically done now. I think if a contributor/maintainer really wants to push on a certain PR that has been inactive for a long time, they should have the flexibility to do so without feeling the need to ping multiple people for explicit permission!

[seberg]

True, I would also prefer this to be a bit less "red tape". Maybe we can split it up into two parts also?
My three thoughts would be:

• 6 months is a very long time for e.g. a tiny doc-fixup PR, but not for some other large ones. I am wondering if we can capture that fuzziness.
• In general, fixing up a PR for a user, or pushing changes seems just generally OK unless the user is working on it very actively (and even then, but it is politer to ask). And I think it is always OK if the fixup is small and a final thing before merging. (I.e. I push a small doc fix and then expect to merge it unmodified)
• The quorum of two makes sense to me for larger PRs, but unfortunately we are missing a bit the system to do it smoothly. For smaller PRs, it seems a bit much red-tape, but it probably doens't matter. Right now, we are probably more in a "close optimistically and reopen on disagreement" mode, although that isn't necessarily great.

[bsipocz]

I would suggest dropping or rephrasing the deemed to be valuable part. In my experience, the value of a PR is just one factor for finishing it for a contributor (if it's not valuable, then I suspect it was already been suggested to be closed, much sooner than waiting for it to get stale).
And no matter how valuable a PR is, if it's complicated or is not almost ready, it's likely won't get taken over.

[charris]

Six months for non-response by the submitter seems reasonable, but I suspect non-response by the reviewers is at least as common :) This also puts a burden on reviewers to track the relevant PRs. I like the proposal in the other comments to implement some sort of automatic tracking and closing. We should pin down how that would operate.

[bsipocz]

Yes, it's a question whether the 6 months become more realistic from the reviewer side of there are only <100 PRs open as opposed to 200+.
How many of those stale PRs are stale because they are lost is the noise as opposed to being complicated, or lack of time from their maintainer etc?
Also, the number of months can be different, I feel the looking at the devstats could help here. E.g is there a time cutoff where a pr is not likely has conflicts? Is there still a good chance for an old pr to get merged?

[InessaPawson]

Also, the number of months can be different, I feel the looking at the devstats could help here. E.g is there a time cutoff where a pr is not likely has conflicts? Is there still a good chance for an old pr to get merged?

@bsipocz Those are excellent questions! We'll keep them in mind considering/designing automation solutions.

[bsipocz]

After today's discussion, I have second thoughts about automation. I still think that the status quo and manual closing is not really sustainable, but letting the bot loose on the currently open ones would likely cause the unwanted alienating effects.

Maybe the automation should first run as a dry-run (the bot can do that), and the output processed into a project board for a heavy triage response?
Maybe the conflicting ones should be picked up on first for a comment (can be semi-automated batch comments on those PRs) asking the contributor to rebase, or if it's clearly close to being ready a maintainer to rebase it and finish the PR straight away?

Either case, I feel it cannot go as fast as the close 70 out of the 1000 issues, or even the close 30 out of the 260 PRs. Those were likely cherry-picked easiest cases, where a clear decision could be made, or the contributor was known, and it was known that alienating wouldn't be an issue.

[bsipocz]

I know you don't necessarily want to automate this process, but overall my experience with bot closing is very positive both from the contributor as well as from the maintainer side. It can be done gently with comments/status changes first then closing after a grace period, as well as with workarounds for PRs that have to stay open.
IMO it removes a lot of subjective factors, and many times also act as a way of reminding about PRs.

[seberg]

I would not mind bot closing to be honest (that is based on e.g. you saying you have positive experience with it). I do wonder if other processes might work as well (i.e. using github projects), but the point is that floating a PR again – a new one – is also not hard and we discussed it now in the newcomers meeting a bit also: It may even help because it gets rid of a lot of baggage.

What helps for me is the positive experience from projects adopting such steps. I don't like "spamming", but it is too bad that 100 stalled PRs tend to hide the 10 PRs that really should't be stalled.

[bsipocz]

Yes. It will be noisy the first time, so ideally will need a few maintainers to be around to make decisions in a timely manner. But after that initial period, it nicely converges to reasonably low traffic.

Here is the bot action: https://github.com/astropy/astropy/blob/main/.github/workflows/stalebot.yml

[mhvk]

Just chiming in to second @bsipocz's comment about the bot: definitely a net positive! The bot warns one it will close something, at which point I usually look at a PR - and I like that if it looks to be too difficult I can just do nothing and let the bot close the PR (obviously, it at times also encourages me to wrap something up...). I'd suggest to copy astropy's setup.

[charris]

But after that initial period, it nicely converges to reasonably low traffic.

I suspect that is how things would go for us. It is a bit like auto-formatting, it would clean things up and then settle down and we wouldn't worry about it anymore.

[seberg]

I might like to add a bullet, but maybe it is distinct (just got to it because of the "finalize" wording):

- Maintainers are encouraged to finalize PRs when only small changes are
  necessary before merging (e.g. code style or grammatical fixes).
  Maintainers may make larger changes when a PR is not actively worked on.
  A PR is a collaboration and sometimes a direct push can help move it
  forward the quickest.

(A bit brainstormish, I admit)

[InessaPawson]

Maintainers are encouraged to finalize PRs when only small changes are
necessary before merging (e.g. code style or grammatical fixes).
Maintainers may make larger changes when a PR is not actively worked on.
A PR is a collaboration and sometimes a direct push can help move it
forward the quickest.

I like your suggestion, @seberg!
Consider a slight rewrite:

Maintainers are encouraged to finalize PRs when only small changes are
necessary before merging (e.g., fixing code style or grammatical errors).
If a PR becomes inactive, maintainers may make larger changes. 
Remember, a PR is a collaboration between a contributor and a reviewer/s, 
sometimes a direct push is the best way to finish it.

[seberg]

Thanks Inessa! It sounds like we have converged and this was discussed in the community meeting, so merging.
If anyone has concerns or would like to tweak the wording please note it (or open a PR).