-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
meta: merge stalebot and update timings/messages #52712
Conversation
Review requested:
|
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
I'm have to refresh my memory in terms of how the endDate helpd us meet our requirements. I think it was to meet what we had agreed in terms of handling what to consider stale. I think https://github.com/nodejs/node/blob/main/doc/contributing/feature-request-management.md will need an update. |
We should also be careful about the impact. We don't want everyone to suddenly have 500 new notifications. |
+1 to what @targos said, I was very careful to avoid that when I introduced it in the first place. |
Yes, that is a lot off issues :-/, WDYT should be done? |
That in part is why I added the end date. If I remember correctly I very carefully moved that a bit at a time so that we would not get a flood of notifications. |
Still, if we look at issues older than a year, it's a large portion of issues, and no matter the end date, floods may still occur. |
After looking at the stalebot docs, there is a parameter https://github.com/actions/stale?tab=readme-ov-file#start-date, so we could only apply this change to newer issues, to prevent the flood of notifications |
Also, in v9.0.0, the stale API now uses the Actions Cache, which will decrease the time it takes to run and the amount of API operations it performs. Based on the issue cache from the help repo's first run of the new stalebot, I estimate that ~209 to ~409 API operations will be spared. |
Bumping, as I believe the stale issues in the repository are far more than they could be.
True, but a few days of heavy notifications in exchange for less stale issues might be reasonable. (Sorry about the review request, didn't mean to do that 😬) |
(requesting review from previous commenters and reviewers) |
Hey, this PR hasn't seen any reviews in months, and it has no objections. Given the gravity of this change, and the lack of reviews, would it be okay if the TSC took a look? I don't want to cause them noise via a ping, so I'd like to ask permission first. |
@RedYetiDev last time when I worked on this, I very carefully managed the issues that were updated to avoid the flood in order to address the concern about a flood of issues. I think if you had an answer to that people might be more willing to move forward. If not then you can ask the TSC to override objections on that front but you would need a specific question and then could add the tsc-agenda tag. My suggestion would be to try to come up with something to avoid the flood, but its up to you how you proceed. |
@mhdawson i don't want to "override" objections, your insight is very important. I've set the operation count to 50, meaning at most, only around 30 issues will be staled a day, does that work? |
@RedYetiDev I don't think the operations do what you want to limit the number that will be closed. Otherwise I think I would have used that originally. |
It's not what they are intended for, but they do (sorta) work like that. If you look at the nodejs/help repo, the operation count used to be 30, and when it was upped to 500, many more issues were closed. You can also have it only apply to newer issues (via a different flag). |
@RedYetiDev have you tested it on your own repo. I'm pretty sure I looked at that in the original pass and it was not a viable option for limiting the issues closed. |
I did some testing. it does limit the issues being closed, but its inconsistent, meaning it (with a (Sorry for the delay in my response, it took multiple days to test) |
I have a separate idea to achieve this while also limiting the impact of this potential change. Once I do testing and verify it'll work non-disruptively, I'll open a new PR. |
This PR will consolidate the
feature-request
and pull request stalebots into a unified workflow that targets all issues and all pull requests.Focusing solely on
feature-request
issues seems unnecessary, as other issues can also become stale.Instead of relying on @mhdawson's fork, which has been great, this PR will update the stale-bot to the latest version (v9.0.0).
However, there's a trade-off. Without the fork, the stalebot loses the
endDate
controlling environment variable. To address this, I've set the stale timeout to a middle ground between theendDate
and the stale timeout (8 months).Previously, the check was for issues older than a year with no updates in the past 6 months. Now, it will check for issues without any updates in 8 months. The middle ground would technically be 9 months, but I think 8 months strikes a good balance in this case.