I’m trying to review the first-time-contribution PRs but I’m trying to do them backwards: Starting from the oldest PRs to the latest.
Is it fine, if I close them if they are stale and have some merge conflicts or should I just write a message? I just did that with this PR and now I don’t know if that was maybe too early 
There is a pr which plan on adding a automated comment for such prs. see: ci: nudge and draft stale merge-conflicted PRs by qweered · Pull Request #534652 · NixOS/nixpkgs · GitHub
Yes, the next step would be to close PRs without an author response some time after the comment is posted. I’ll work on that soon. With such high amount of incoming PRs we should definitely close long time stale and merge-conflicted ones
It has been my experience that stale PR tracking tends to be way too aggressive about assuming that someone has lost interest. Automated “hey, we’re waiting for your input” pings at, like, monthly intervals are fine, but if it were my call, I would give a PR at least two full years of complete lack of response before giving up on the contributor.
Also, PRs should never, ever be closed automatically, except for spam filtering.[1] Outside of spam, someone went to the trouble of making and submitting the PR. The very least the project owes them is a human writing a response explaining why their contribution is being declined. This isn’t just a matter of courtesy; someone who’s nervous about their first contribution to a new project may well take an automated “no one has looked at this in three months so it’s being closed” response as code for “we don’t want your help, go away forever”!
(All this goes triple for bug reports, incidentally.)
AI-slop contributions are, at this point, best treated as spam IMNSHO ↩︎
I would give a PR at least two full years of complete lack of response before giving up on the contributor.
imo 2yrs is WAY too long… i’d say 6mo is more reasonable
To toss in my two cents, I agree that PRs shouldn’t be automatically closed. But I also think that PRs with any useful work in them shouldn’t be manually closed either – as long as there’s a reasonable amount of salvageable code, I think they should stay open until they are either superseded by a new PR or become unsalvagable.
So in this case, node-packages is now removed and nothing else useful remains, so it’s definitely fine to close. But in a scenario where a PR is adding a new package, and maybe the package needs to be moved to a different pname but is otherwise suitable for inclusion, I think it should stay open until someone else interested finds it and reuses the work in their own PR, the package becomes unmaintained upstream, etc.
Of course that doesn’t mean that PRs can’t be manually closed because e.g. the package doesn’t meet the requirements or the technical approach is unsound. Just that they shouldn’t be closed solely because of staleness.
That’s just my non-committer opinion though.
@StepBroBD The stale label appears after ~6 month of inactivity (180days). We would match more that 2000 prs. With a 2 year window, it matches ~110, 97 of which have also merge conflicts.
I do agree with @GraysonTinker that we should avoid closing prs lightly, after all we have written in our STALE-BOT.md: “Our stale bot will never close an issue or PR.”. Closing old prs also requires getting the context to make a proper decision, which can be also time consuming, and its not time spent on reviewing our very active backlog that keep growing.
I noticed that one of the latest changes on the stale bot config was to disable stale bot comment on inactive prs (personality i think that the comment approach was better than an easy to miss description on the stale label ).
I disagree in the linked PR description (ci: nudge and draft stale merge-conflicted PRs by qweered · Pull Request #534652 · NixOS/nixpkgs · GitHub) it explicitly states that only PRs with an merge conflict and no activity are targeted.
I was in this situation just a few weeks ago. I created my first PR for nixpkgs and wanted it to be merged or feedback on what problems are. If i had gotten an automatic response “no one has looked at this in three months so it’s being closed” you are absolutely right that i would have been dishearten and wouldn’t work on my second PR now. As it only targets PRs with merge conflicts and notifies the creator before closure. I think it is reasonable to close the PR if the creator can no longer be bothered to fix the merge conflict. I don’t want to discredit the work that went into the PR but as a project nix needs to draw a line somewhere to lighten the load of reviewers and if the creator can no longer be bothered to resolve the merge conflict i find it reasonable to close the PR especially as the creator would have the burden of updating the software to new versions and if he doesn’t resolve a merge conflict i don’t believe that he would keep the package up to date.
Damn, I didn’t expect to start such a huge discussion xD
Since there seems to be a PR for auto-closing stale PRs I think it’s fine to ignore those then and let the upcoming bot do it.
My take is that reviewers probably are already filtering out theses prs in some way (by filtering out the stale/merge label, or just sorting by most recent prs). So I don’t think it would really lighten anything
Our current reviewing bottleneck seems to be that we are receiving more pr that we can deal with every month causing them to accumulate. This is a what will create many more stale pr in the long run.
If you look at Pulse · NixOS/nixpkgs · GitHub, you can see that out of the 11182 pr we received, only 9031 were closed, and 2151 are still open. In the last 6 month, we went from ~9600 pr open to ~12k which represents a +400pr a month
Yeah, I asked in the saltsprint what I could do, to help people with commit/merge permission to simplify the reviewing process. They told me to help first-contribution PRs which I’ll try to do 
Maybe slightly off topic, but it seems like github nested search with parentheses is broken. Maybe I’m holding it wrong? I’m trying to use Github search to understand the makeup of the open PRs.
is:pr is:open label:"8.has: package (new)" returns 2,728 open PRs, but is:pr is:open (label:"8.has: package (new)") returns 0 open PRs.
What I wanted to do was something like is:pr is:open (label:"8.has: package (new)" OR label:"8.has: package (update)") to find out how many PRs were clearly nixpkgs package expression updates vs. how many were e.g. module or library updates.
Yeah reducing time to reply for new contributors is important!
What’s the point in closing old PRs or issues anyway? There’s no way nixpkgs will ever get to an inbox-zero, and I don’t see any benefit to reaching that point anyway. If it ever does happen we’ll be back to hundreds within a week, there is no chance of a humanly-manageable open issue number.
Stale bots are known to be nothing but annoying. There are so many projects where I have an issue or two on which I just add weekly pings to keep a genuine issue nobody has the spoons to solve open. Please don’t add nixpkgs to my list of bot-fighting repos.
For issues I definitely agree!
But for PRs that have open change requests and discussions, that haven’t been iterated on by the original PR author, and by today wouldn’t even be mergeable anymore due to conflicts, just let us close those after 2 release cycles.
PRs definitely seem like a lesser evil, but still: what’s the motivation for closing them in the first place?
I see no benefits, only the same (albeit, I agree, much less severe) risk of alienating contributors that closing issues has.
And even if that is either not something we care about or we deem the risk low enough, why do pointless busywork, even in automated fashion? There will always be too many PRs, it doesn’t help anyone. I strongly believe there should be a clear motivation for something like this.
An old PR with merge conflicts can simply be rebased, someone else can make a new PR based off it and write closes #[old pr] in the desc which will close the old PR by virtue of merging the new one (and then adequately cite it as well). How long to wait before creating a new PR is up for debate, see Policy needed for PRs that are seemingly abandoned by their author · Issue #427163 · NixOS/nixpkgs · GitHub for additonal context.
The main reason to simply outright close an old PR would be if it doesn’t make sense anymore, but that would apply even for a new PR; age is not even a relevant factor.
I’d like to add the following thought:
It doesn’t seem fair to me to ask people to resolve conflicts if there’s no more than a slim theoretical chance they’ll get a review or even merge afterwards.
Otherwise, it’s just an unnecessary burden for them, and the fact that they didn’t receive a review or merge in the first place is simply brought up again, which causes even more frustration with the situation and, in my opinion, is neither productive nor fair.
It might also be worthwhile here to not just say “this PR seems stale" but also add links to resources to ask for a (re)review/merge. (Like discourse)
Someone new to the project is likely not familiar with the social channels available for getting attention for a code review. Getting reviews can be especially hard for new package/module contributions.
The entire “stale bot” has been controversial for a long time. In its current form it merely adds noise and annoyance. Personally, I rather see it gone completely.
It would be nice if it just shared a link with where to get help with reviews, but then it would still be noise for everyone that is not a newcomer (which is most PR’s/issues). So perhaps only for PR’s that have the newcomer label and hasn’t had a review for some time? But that still doesn’t solve the very basic issue that we have too few reviewers.