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

github: add configuration for stale-bot #11921

Merged
merged 1 commit into from
Aug 9, 2019

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jul 26, 2019

Contribution description

This adds a configuration for the probot stale app. It basically implements what we already do by hand / with comments: If there is no activity in an issue/PR for > 1/2 year, mark it as stale (using the new State: stale label) and if there is still no activity then, close it. Excempt from this are the issues/PRs marked with the following labels:

  • Area: security
  • State: demonstator
  • State: don't stale
  • Type: tracking

Maybe this somewhat increases the productivity.

Testing procedure

Not sure this can be tested without the file being merged :-/.

Issues/PRs references

None.

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: tools Area: Supplementary tools labels Jul 26, 2019
@miri64 miri64 added this to the Release 2019.10 milestone Jul 26, 2019
@MrKevinWeiss
Copy link
Contributor

I like the idea, it would also bring PRs that might have been forgotten but are still good back in the light.

@miri64 miri64 added the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label Jul 26, 2019
@miri64
Copy link
Member Author

miri64 commented Jul 26, 2019

I think it also would be friendlier if a bot would warn about stalement and then close it instead of some random commenter ;-).

@jcarrano
Copy link
Contributor

I've been thinking on something like this for a while. It will be a good thing. Also people tend to be more tolerant towards bots than people- if a bot closes your issue you won't be as annoyed.

@leandrolanzieri
Copy link
Contributor

leandrolanzieri commented Jul 26, 2019

I really like the idea, I think it will be really useful. If this gets merged, will it automatically apply the label for all the PR that are already open (and match the criteria)?

@miri64
Copy link
Member Author

miri64 commented Jul 26, 2019

I really like the idea, I think it will be really useful. If this gets merged, will it automatically apply the label for all the PR that are already open (and match the criteria)?

The doc is not clear on that. I guess so.. But I am not sure :(

@fjmolinas
Copy link
Contributor

I really like this idea. My only question is about the exempt labels list, maybe other labels should be added. I'm not sure if its good for bugs issues to get automatically closed, not in all cases at least, weird border cases that might have been hard to find might get covered up again if the issue is closed. Maybe a new label known issues can be added, for issues like #495 for example; so those ones don't get closed.

@miri64
Copy link
Member Author

miri64 commented Jul 29, 2019

I really like this idea. My only question is about the exempt labels list, maybe other labels should be added. I'm not sure if its good for bugs issues to get automatically closed, not in all cases at least, weird border cases that might have been hard to find might get covered up again if the issue is closed. Maybe a new label known issues can be added, for issues like #495 for example; so those ones don't get closed.

I think it is a good idea to remind people every half a year or so, that there is still a bug issue open. If they are on the exempt list they will be completely ignored by the bot, which would not have the desired effect. Remember: the closure only happens after 7 more days of inactivity.

@miri64
Copy link
Member Author

miri64 commented Jul 29, 2019

Remember: the closure only happens after 7 more days of inactivity.

And if after that time the bug still isn't at least acknowledged, it is probably not worth being kept open ;-)

@fjmolinas
Copy link
Contributor

I think it is a good idea to remind people every half a year or so, that there is still a bug issue open. If they are on the exempt list they will be completely ignored by the bot, which would not have the desired effect. Remember: the closure only happens after 7 more days of inactivity.

I agree, having the bot ping about the issue is always useful. But those are different functionalities, (a) a bot the pings people on inactive stuff, (b) a bot the closes inactive issues/PR's. I had a quick search online and didn't find something that would split both functionalities.

And if after that time the bug still isn't at least acknowledged, it is probably not worth being kept open ;-)

Well that depends, the involved maintainers might not be active anymore or on vacation, and it could get closed because no one is actually getting pinged (i'm just referring to the 7 day closure window here, 1/2 year to become stale is more than enough). Buy anyway as a general rule, you are right, an I'm only pointing out border cases to be aware off.

@fjmolinas
Copy link
Contributor

I really like the idea, I think it will be really useful. If this gets merged, will it automatically apply the label for all the PR that are already open (and match the criteria)?

The doc is not clear on that. I guess so.. But I am not sure :(

If it automatically tags all stale issues/PRs as soon as is get merged and then closes them all in 7 days a lot of un-intended closures could happen (specially with so many people on vacation right now).

@miri64
Copy link
Member Author

miri64 commented Jul 29, 2019

Well that depends, the involved maintainers might not be active anymore or on vacation, and it could get closed because no one is actually getting pinged (i'm just referring to the 7 day closure window here, 1/2 year to become stale is more than enough).

If a week of absence of one maintainer is enough to cause chaos, we seriously need to rethink our processes. However, I think the case you draft out is more hypothetical than actually bound to happen. Also, if this really would happen, remember that the issue is only closed, not deleted. The maintainer in question can still reopen it after their leave of absence.

@miri64
Copy link
Member Author

miri64 commented Jul 29, 2019

If it automatically tags all stale issues/PRs as soon as is get merged and then closes them all in 7 days a lot of un-intended closures could happen (specially with so many people on vacation right now).

We don't have to merge it now ;-). And I think the people still their can also make a call of judgement and keep those issues open...

@miri64
Copy link
Member Author

miri64 commented Jul 29, 2019

We could also make the period between staling and closing longer, e.g. 31 days... It rarely happens that a person is on vacation or sick for 31 days and even then I hope contingency plans were made.

Also, if there is a major bug in a feature that was not fixed for 6 month and nobody feels like at least commenting on that issue (even so much as writing "pong"), we should rather think about removing the feature...

@fjmolinas
Copy link
Contributor

fjmolinas commented Jul 29, 2019

Just as a comment, one can check the issues/PR that would get tagged by adding updated:<2019-01-29 to the search, that would mean:

  • 345 issues
  • 161 pr's

Which doesn't seem that bad for the PR's at least. Note that a lot of issues don't get tagged as stale just because they are bumped to future releases milestones, with no actual change happening on those PRIssues.

@miri64
Copy link
Member Author

miri64 commented Jul 29, 2019

Which doesn't seem that bad for the PR's at least... Note that a lot of issues don't get tagged as stale just because they are pumped to future releases milestones, with no actual change happening on those PR's.

Not sure if this is count as activity or not by the stale bot. We have to see.

@fjmolinas
Copy link
Contributor

Not sure if this is count as activity or not by the stale bot. We have to see.

According to the doc it does:

The app uses GitHub's updated search qualifier to determine staleness. Any change to an issues and pull request is considered an update, including comments, changing labels, applying or removing milestones, or pushing commits

@miri64
Copy link
Member Author

miri64 commented Jul 29, 2019

According to the doc it does:

The app uses GitHub's updated search qualifier to determine staleness. Any change to an issues and pull request is considered an update, including comments, changing labels, applying or removing milestones, or pushing commits

Must have missed that. So maybe go down to one release cycle (~100 days)? Or did you just wanted to point this out?

If it automatically tags all stale issues/PRs as soon as is get merged and then closes them all in 7 days a lot of un-intended closures could happen (specially with so many people on vacation right now).

If a PR is merged it won't get stale because it is already closed, so I have trouble understanding this sentence ^^"

@fjmolinas
Copy link
Contributor

If a PR is merged it won't get stale because it is already closed, so I have trouble understanding this sentence ^^"

Sorry, I wrote it in a confusing way hahaha. I was referring to when this PR gets merged, i.e. the bot becomes operational. If that means that 345 issues and 169 PR's would become stale and potentially closed in the next 7 days, it might be worth sending an email on the mailing list after or before merging.

Must have missed that. So maybe go down to one release cycle (~100 days)? Or did you just wanted to point this out?

I just wanted to point out that the amount of stale issues is probably bigger. That being said, trimming it down to a release cycle makes sense to me.

@miri64
Copy link
Member Author

miri64 commented Aug 5, 2019

Updated to current state of discussion here and on the maintainers mailing list.

@kaspar030
Copy link
Contributor

I'm inclined to ACK this. The bot will mark ~500 issues/PRs as stale right away, right? That'll be a couple hundred notifications for everyone. 😉

@miri64
Copy link
Member Author

miri64 commented Aug 9, 2019

I'm inclined to ACK this. The bot will mark ~500 issues/PRs as stale right away, right? That'll be a couple hundred notifications for everyone.

I don't know actually... We'll see what happens. I don't believe it will be that many however. Many PRs and issues changed labels quite recently when we updated our labeling system and milestones were also changed for the last release.

@kaspar030
Copy link
Contributor

We'll see what happens.

OK! 😉

Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. Please squash!

I'd be very happy to have a couple more ACKs to not take the responsibility on my own.

@miri64
Copy link
Member Author

miri64 commented Aug 9, 2019

Squashed. IIRC @MrKevinWeiss wanted to have another look as well.

@aabadie
Copy link
Contributor

aabadie commented Aug 9, 2019

Like many others, I wanted to introduce this kind of tool but I didn't this one already exists.

I like the approach of this PR. Let's try it. ACK

Copy link
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! ACK!

@miri64
Copy link
Member Author

miri64 commented Aug 9, 2019

@fjmolinas you had some concerns. Are you alright with merging this?

@fjmolinas
Copy link
Contributor

@fjmolinas you had some concerns. Are you alright with merging this?

The important concerns have been addressed, or are no longer valid. We can see if reducing daysUntilStale to a release cycle makes sense once in production. So lets go ahead and give it a try, ACK.

@kaspar030
Copy link
Contributor

Then let's go. :)

@kaspar030 kaspar030 merged commit 7f857ca into RIOT-OS:master Aug 9, 2019
@kaspar030
Copy link
Contributor

Is probot already activated for our repo?

@miri64
Copy link
Member Author

miri64 commented Aug 9, 2019

Is probot already activated for our repo?

yepp

@miri64 miri64 deleted the github/enh/stale-bot branch August 9, 2019 13:05
@kaspar030
Copy link
Contributor

Is probot already activated for our repo?

yepp

is there some kind of status or output page?

@miri64
Copy link
Member Author

miri64 commented Aug 9, 2019

@aabadie
Copy link
Contributor

aabadie commented Aug 9, 2019

@miri64 I get a 404 when following the link.

@miri64
Copy link
Member Author

miri64 commented Aug 9, 2019

You probably need to be in Team Admin to see it but apparently @kaspar030 isn't either. Maybe one of the members of @RIOT-OS/admin can confirm?

@kaspar030
Copy link
Contributor

Maybe one of the members of @RIOT-OS/admin can confirm?

I can see that probot is enabled!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants