-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
GitHub: auto set inactive label #25163
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
Conversation
e6e4712
to
fdf81a4
Compare
Obviously I approve of this sort of thing, but I thought @melissawm was working on something? |
I didn't see anything like that, but happy to close and let someone else put a PR in. |
This would close like 1200 issues. Not saying that it is a bad idea, but some of those are clearly bugs or "surprising features". Maybe we should have some labels that does not auto-close? Confirmed bug at least? 76 would be closed status: confirmed bug (at least based on opening date, probably a bit fewer in reality). Edit: Also, I think that at least some of the feature requests should stay. Even though the easy ones often gets picked up quite quickly, it is a bit of a shame to lose good suggestions just because no one had time to implement them. (I think people may not really bother to reopen them.) The problem is most likely that we then would have to decide on "worthwhile feature requests"... Btw, are these closed as "fixed" or "not planned"? |
We currently have 871 open issues that have not been updated for a year of which 49 were confirmed bugs so this proposal would mark those as inactive. They would only be closed if nobody comments on them for a further month after they were labelled. I expect that some of these bugs have actually been fixed, so I would not exempt this label. Rather, I would expect that someone who is watching the issue will see the notification from the bot's comment and check whether a recent mpl version still has the problem. If it does, they can comment to say so and the issue stays open for another year. |
I think the same for feature requests - if anyone still wants the feature, they have 30 days to comment. We will also all see it in our notifications and can comment and perhaps reclassify or act on it as maintainers. If no one even comments, then the request probably wasn't that important or has workarounds. |
Thanks @jklymak ! A couple of comments:
|
Sure that wording is great. For issues, I think we could even be more explicit why we are doing this so people don't take offence. I added
See the uploaded commit with slight modifications to the above. Also, note the time frames on these are just my first guess at what we want. Super easy to change... 30 d close seems long enough to catch anyone who went on vacation. 365 days seems like a good length of time to expect an issue to have been looked at and acted upon (somehow). PRs should really not sit around for two months, in my opinion at least, particularly if it is a non-core dev we should be making it clear they are welcome to ping us. |
e1e75ce
to
6b3f5df
Compare
For PRs, I think it would be good to find a form of words that encourages the author to at least let us know whether they plan to continue working on it. I worry that "you can pick it up anytime" leaves it open to drift indefinitely. |
" If you do not plan on continuing the work, please let us know and we will find either find someone to take the PR over or close it" ?? |
6b3f5df
to
1449758
Compare
very slight tweak:
|
1449758
to
f2a10d1
Compare
Note that I've also reduced the operations per run to 5. The idea being that this will lead to fewer notifications per day, and as this comes on line we may actually respond to them. We may need to play with this or the cron frequency depending on how many notifications we get. |
jobs: | ||
stale: | ||
runs-on: ubuntu-latest | ||
steps: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you also add an inactive project? my big reason for that is it lets us potentially mark up why stuff is stalled in a private way, which may be helpful for some more contentious stalled things. https://github.com/actions/add-to-project
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you follow up with that? I have no idea how to do that, and I think can be completely independent of this PR.
The action sifts through issues/PRs in chronological order and by default starts at the newest. I suggest setting It also doesn’t remember where it got to last time it ran, so I think after the first several runs it will use up its operations just re-checking issues/PRs that it already checked. So I think we would need to gradually increase the operations per run to help it get further. |
I don't understand how it could possibly work then if it has no way of knowing what's already been done?? The default query is 30. |
Sorry @jklymak for editing your comment when I was trying to reply. Is there a way to filter so that the action is only run on issues not labeled inactive. |
OK, I think it works by downloading issues/PRs 100 at a time, and then doing operations on them. I think each 100 takes an action. Then each operation takes an action. So we have about 2000 open issues+PRs, so that will use at least 20 actions leaving 10 actions to set labels and/or close things. So maybe that is not too bad left at 30? |
I'm pretty sure thats how it works. |
If it finds the “inactive” label it then checks whether there was a comment since the label was added. If yes, it removes the label. If no, it checks whether the “days before close” time is up and closes if so. Disclaimer: all my comments are based on having observed the behaviour in Iris - I haven’t actually studied its inner workings! |
not-planned, by default... |
actions/stale#792 seems relevant. Perhaps we need to do some of this offline first with a job that can store its state. I'm still not sure from this what constitutes an "action" on the GitHub API that counts against the counts. https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#rate-limits-for-requests-from-github-actions seems to indicate that from GH actions we could use 1000/h. But if that is still trying to get through 2000 PRs/issues, it's still not going to succeed. |
I also don't know what counts as an "operation", but maybe it doesn't matter if it can't get through all 2000: after the first month, it should start closing some, so then there are less than 2000 to get through. If we also want to keep notification frequency down, we probably don't want it to find all ~900 inactive items in the first month. |
3c075e2
to
21fea65
Compare
.github/workflows/stale.yml
Outdated
days-before-issue-stale: 365 | ||
days-before-issue-close: 30 | ||
ascending: true | ||
exempt-issue-labels: keep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added exempt labels (which could be expanded) so that if we have things we don't want cleaned up we can "keep"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the "Orphaned PR" label should be exempt, as it seems redundant to check those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Not sure if I have the syntax right for the labels with spaces in them...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This format seems to work in Iris
https://github.com/SciTools/iris/blob/main/.github/workflows/stale.yml#L67
Should the repo token be set like it is for PR welcome?
I admit that I don’t know the difference between this and the default used by the action. https://github.com/actions/stale#repo-token It might also be good restrict to this repo so it doesn’t run on forks, like we do for the tests: matplotlib/.github/workflows/tests.yml Line 31 in 91d8f2e
|
Sure, did both of those. No idea what the effect is so maybe someone with more GH Actions experience should check? |
47f6f69
to
eff767a
Compare
I killed appveyor for this PR to un-block running a higher-priority one for 3.7.0 prep. |
@rcomer, can I ping you for review? Or we should close this if we aren't actually going to do it.... |
I think @ksunden said he would take a look at this. I'm far from an expert in GitHub Actions. |
Functionally, my only comment is the space in the comma separated label list. Though I do want to revive @melissawm's request that these timelines appear in the docs. |
Agreed, but I think it's premature to document all this yet until we figure out if/how it works? |
Co-authored-by: Kyle Sunden <git@ksunden.space> Co-authored-by: Ruth Comer <10599679+rcomer@users.noreply.github.com>
d5fef19
to
ddaa7ce
Compare
squashed and pushed to kill CI, which doesn't do anything for this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine to go in based on one (@ksunden) technical review - it’s not like it’s user facing code. The messaging has had input from several people. My only question is whether we should now leave it a couple of days before merge, so that notifications don’t pile up over the weekend.
This is the thing. I do not expect most users to check five or ten year old bug reports and, if the bug still exists, write something kind/sensible. Also, keeping track of notifications is quite hard for many people. A few days off and it is impossible to not miss something. I'm quite sure we will lose sensible reports and feature requests. Finally, do we know if these are "closed as completed" or the other option? Ideally it should be a third option, "hide", for these, so one can check back in case one decides to work on a particular thing. |
I think the worst that can happen is that a currently valid issue gets closed and forgotten. But the situation now is that we have nearly 1600 open issues, and I'm not sure it's realistic to manually sort through them all to see which are still relevant. So either way, issues are forgotten. If a bug or feature request affects a lot of users, then I would expect more than one user to be subscribed to the issue. That should reduce the risk of the notification being completely missed. Also devs can see the bot's updates and can make our own judgements if we think something should stay open. @jklymak has deliberately chosen settings to keep the frequency low and therefore hopefully manageable.
They are closed as "not planned", but will also retain the "inactive" label, so could be searched that way. |
I just created the “inactive” and “keep” labels. Currently in white. Please update colour and description as you see fit! |
Well, that worked - caught 8 old issues (no PRs) that it marked inactive. I marked one of them "keep". We will see if it moves on from there.... |
I guess the oldest PR is newer than a lot of issues so it will take a long time to get to any PRs. Maybe I shouldn’t have suggested to set |
I've checked the eight and am tempted to say that they are all valid requests (not sure if the colormap one is still an issue though). Not sure if I should mark the non-marked ones though... What about adding a "closed but relevant" tag? In that way we can only keep "active" issues, but still access unsolved issues when needed. Like the mathtext issues in case we get a GSoC student working on it? |
Maybe in a month's time when things start to close, we could review what was closed and what we think about it. Certainly I see no downside to adding whatever labels you think are useful to the closed issues. |
There is "valid" and then there is "reasonable". If an issue is open for 12 y folks have clearly been making do without the request. But for sure, if anyone thinks an issue will get acted on, they should comment or mark as "keep". |
PR Summary
Proposed stale action...
It labels PRs after 60 d, but never closes them.
It does close issues after 1 year with 30 day warning.
PR Checklist
Documentation and Tests
pytest
passes)Release Notes
.. versionadded::
directive in the docstring and documented indoc/users/next_whats_new/
.. versionchanged::
directive in the docstring and documented indoc/api/next_api_changes/
next_whats_new/README.rst
ornext_api_changes/README.rst