Skip to content
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

Use stale bot for commenting on stale issues/PRs in Node.js core repo #28798

Closed
trivikr opened this issue Jul 21, 2019 · 29 comments
Closed

Use stale bot for commenting on stale issues/PRs in Node.js core repo #28798

trivikr opened this issue Jul 21, 2019 · 29 comments
Labels
blocked PRs that are blocked by other issues or PRs. meta Issues and PRs related to the general management of the project.

Comments

@trivikr
Copy link
Member

trivikr commented Jul 21, 2019

Is your feature request related to a problem? Please describe.
Comment by @bnoordhuis in #25209 (comment): we have a long (loooong) backlog of probably-stale issues that no one is looking at, with many where discussion has meandered so much that no one is really sure anymore what they're even about.

Describe the solution you'd like

Describe alternatives you've considered
Manually finding out stale issues and commenting on them, like @targos did in #25209 (comment) 😜

@targos targos added the meta Issues and PRs related to the general management of the project. label Jul 21, 2019
@sagitsofan
Copy link
Contributor

sagitsofan commented Jul 21, 2019

+1

@bnoordhuis
Copy link
Member

I still think it's a good idea. It's not as if the number of open issues has gone down since last time, quite to the contrary.

@Trott
Copy link
Member

Trott commented Jul 24, 2019

@nodejs/tsc

@Fishrock123
Copy link
Contributor

we have a long (loooong) backlog of probably-stale issues that no one is looking at

I do try to semi-routinely go through the issues backlog oldest-first.

I very much dislike auto-closing old issues, personally.
Maybe that's just because I do triaging weirdly, I don't know.

To me, the reality is that there are going to be a good chunk of unresolved things and, that's ok. At least it's a clear signal that any given issue there wasn't resolved and may be picked up.

Also, all things considered, I think we still do a pretty good job for the size of project we are.

@sam-github
Copy link
Contributor

sam-github commented Jul 24, 2019

Just to be clear, most of the comments quoted in the description are in favour of closing stale issues, not old issues. Stale would be issues with no activity.

If a ping is published first to notify of upcoming auto-close, it should only take someone replying with "pls keep open" to make it unstale for some time (I'd assume it'd be a couple more months).

If no one can be bothered to post a "please don't close this" into an issue every few months... then I'd call it a good candidate for closing. Closing doesn't mean it can't be reopened on request, or commented on, or searched, or anything really. Primarily, it cuts down the number of things that have to be looked at when the open issues are periodically sieved through by someone generous with their time.

@mhdawson
Copy link
Member

My cents would be:

  • base it on x time since last activity in the issue
  • Ensure the message added on close makes it clear we want those involved to re-open if it should still be open
  • Provide a tag that can be used to tag issues that we think are ok to stay open with out activity for a long time. For example (although not a good example necessarily) it may be expected/ok for features requests we've reviewed and agree with to stay open even if there currently no ongoing activity. Backports might be another one, were some will sit waiting for the next SemVer minor release.

Seems like stale bot should be able to be configured along those line.

@trivikr
Copy link
Member Author

trivikr commented Jul 24, 2019

The default configuration for staleBot is given in their README

Based on suggestions from @mhdawson:

base it on x time since last activity in the issue

  • it's defined in daysUntilStale and daysUntilClose

Ensure the message added on close ...

  • the comment can be added in markComment

Provide a tag that can be used to tag issues that we think are ok to stay open with out activity for a long time.

  • the option is available under exemptLabels
  • there's also onlyLabels, exemptProjects, exemptMilestones, exemptAssignees which we can use if required

Backports might be another one, were some will sit waiting for the next SemVer minor release.

  • backports can be done even after issue/PR is closed right?

@trivikr
Copy link
Member Author

trivikr commented Jul 24, 2019

Suggestion from @sam-github:

If a ping is published first to notify of upcoming auto-close, it should only take someone replying with "pls keep open" to make it unstale for some time (I'd assume it'd be a couple more months)

Their app states under point 2:

If the Issue or Pull Request is updated, or anyone comments, 
then the stale label is removed and nothing further is done 
until it becomes stale again.

So any activity on issue/PR will unmark the issue as stale

@trivikr
Copy link
Member Author

trivikr commented Jul 24, 2019

Suggestion from @Fishrock123:

I very much dislike auto-closing old issues, personally.

  • We can set a big value for daysUntilClose (like 3 or 6 months)

To me, the reality is that there are going to be a good chunk of unresolved things and, that's ok. At least it's a clear signal that any given issue there wasn't resolved and may be picked up.

  • We can use use exemptLabels tags for such discussions

@mcollina
Copy link
Member

@mhdawson stalebot does all of that. I would configure it so that an issue/pr become stale after 6 months, and get closed automatically after 1 year? This aligns it with our release cycle: essentially if it did not make it in 2 majors, then it likely won’t happen at all.

@trivikr
Copy link
Member Author

trivikr commented Jul 25, 2019

The stale comment is just to notify subscribers that issue has been stale for a while (time of inactivity). Six months could be little long for that.
Can we reduce the time of inactivity?

@bnoordhuis
Copy link
Member

Empirically, I'd say that > 50% of issues that have been dormant for a month, stay dormant.

@trivikr

This comment has been minimized.

@trivikr
Copy link
Member Author

trivikr commented Jul 29, 2019

As described in comment probot/stale#224 (comment), it looks like stale bot will comment on issues without staleLabel and remove staleLabel once there's any activity outside of stale bot.

Empirically, I'd say that > 50% of issues that have been dormant for a month, stay dormant.

How about these values?

  • daysUntilStale: 60
  • daysUntilClose: 365

The stale bot will comment on all issues without staleLabel after 60 days of inactivity, and close stale issues after 365 days of inactivity.

@trivikr
Copy link
Member Author

trivikr commented Jul 29, 2019

I've created a feature request with stale bot to not include staleLabel while getting stale issues at probot/stale#225

If this feature request is implemented, stale bot will continue commenting after daysUntilStale of activity even if staleLabel is added.

@trivikr
Copy link
Member Author

trivikr commented Aug 2, 2019

If there's no more update, I'll post a PR over next couple of days for stale bot config with following values:

  • daysUntilStale: 60
  • daysUntilClose: 365

Values for other configurations can be discussed in the PR comments

@targos
Copy link
Member

targos commented Aug 2, 2019

For reference, here are the permissions requested by the Stale app:

image

@targos
Copy link
Member

targos commented Aug 2, 2019

@trivikr do you know what will happen to existing issues after we enable it?
Are we all going to get hundreds of notifications at the same time?

@trivikr
Copy link
Member Author

trivikr commented Aug 2, 2019

@targos I recently created config for lock bot (a different bot), which has a config called skipCreatedBefore which skips issues and pull requests created before a given timestamp (docs)

I don't see any such config for staleBot (docs)
I've created a request at probot/stale#227, and added block label for this issue

@trivikr
Copy link
Member Author

trivikr commented Aug 2, 2019

Stale bot has a config called limitPerRun which defaults to 30
If we introduce stale bot now, I expect to get 30 notifications per hour till all stale issues are marked.

One way we could avoid this is to manually mark issues with staleLabel so that stale bot doesn't comment on them, but they'll be resolved if they are more than daysUntilClose (i.e. 365 days for us) old.

EDIT(trivikr): typo still > till

@devsnek
Copy link
Member

devsnek commented Aug 2, 2019

I think it would be better to not limit the bot to a certain date range. Maybe we can set the limitPerRun lower, but we definitely should go through all our issues. At 30/hr it will probably take between 10/20 hours, which sounds reasonable to me, but if that seems overwhelming we can lower it to like 5/hr.

@trivikr
Copy link
Member Author

trivikr commented Aug 2, 2019

The problem with too many stale bot notifications in the beginning is that some subscribers might miss genuine notifications if they clear all of them. Also, they might raise complaints after getting too many notifications.

The idea with skipCreatedBefore config was to add some date now, and reduce the date by some interval (say 3 months) regularly (say once every other week) so that:

  • stale bot notifications are spread over time
  • subscribers get used to stale bot notifications

For example, we start with value 2019-04-01 to skipCreatedBefore:

  • two weeks from now, reduce it to 2019-01-01
  • two weeks from then, reduce it to 2018-10-01
  • two weeks from then, reduce it to 2018-07-01
  • and so on...

Instead of fixed values (like three months), we can also update skipCreatedBefore based on number of issues/PRs which are stale (say 50 in one go) - with max 5 notifications each spread over 10 hours.

Each config update can be merged when subscribers won't mind notifications (say weekend?)

@trivikr
Copy link
Member Author

trivikr commented Aug 8, 2019

GitHub announced Actions Beta today, which has an action for posting warnings and closing stale issues and PRs

@bnoordhuis
Copy link
Member

Are we moving forward with this? We're edging towards 800 open issues...

(Remember when we had only 500 open issues? Good times.)

@Trott
Copy link
Member

Trott commented Aug 20, 2019

Are we moving forward with this? We're edging towards 800 open issues...

(Remember when we had only 500 open issues? Good times.)

Aside: Hilariously, I've been trying to keep the number of open PRs down below 300 and was successful for a while, but now it all feels so hopeless. If you have an open PR that is realistically never going to be finished, please do me a favor and close it!

@trivikr
Copy link
Member Author

trivikr commented Aug 20, 2019

Summary of configs discussed in this issue:

The issue created with stalebot at probot/stale#227 hasn't receive any responses.
There's an experimental PR in Node.js core to use GitHub actions CI for running tests at #29193

I've created new issue at #29232 specific to writing a stale action, as the discussions in this issue were mainly related to stale bot

@bnoordhuis
Copy link
Member

Are we moving forward with this? We're edging towards 800 open issues...

We're over 900 open issues as of today. Can I suggest we move forward with either this issue or #29232?

@jasnell
Copy link
Member

jasnell commented Jan 24, 2021

Does this need to remain open?

@trivikr
Copy link
Member Author

trivikr commented Jan 25, 2021

Does this need to remain open?

Nope, we should go ahead with GitHub Actions discussed in #29232 instead.

@trivikr trivikr closed this as completed Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked PRs that are blocked by other issues or PRs. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests