elsa-workflows / elsa-core

A .NET workflows library
https://v3.elsaworkflows.io/
MIT License
5.95k stars 1.09k forks source link

Stale Bot #697

Closed jwillmer closed 3 years ago

jwillmer commented 3 years ago

It does not matter if you merge first or after you approved the bot since stale bot will wait 24h before the first run.

https://github.com/probot/stale

craigfowler commented 3 years ago

There's already a discussion which touches on this here https://github.com/elsa-workflows/elsa-core/discussions/688

We might not need the Stale Bot when we get around to implementing that process. My only concern about Stale bot is that it could end up closing issues in our backlog, just because they're not going to be implemented any time soon. I can foresee myself 'fighting' a tool like this and re-opening a lot of what it closes, because the issue contains valuable information about a bug or wishlist feature which should be in the backlog.

Triage and issue grooming is something we need to get on top of, but I'm not sure that "close any issues which have no activity since X time" is the right solution yet. I think that (eg: using the process suggested in the linked discussion) we could actually get out in front of our pile of issues, without discarding work/contributions which have already been made.

craigfowler commented 3 years ago

Also, I notice this breaks the build but that's really my fault more than anything. If you cherry-pick (or just copy the same change) made in https://github.com/elsa-workflows/elsa-core/commit/850b01b2d25372c1060c67d9d1095b0621c9bec6 then it'll fix the build. Because of the secure variable (which is not evaluated on a PR build), the SonarScanner tries to run but fails on an invalid API token. The only fix I could find was not to run SonarScanner for PRs.

jwillmer commented 3 years ago

@craigfowler I configured the bot in a way that it does not close any issue that has already a label assigned (except wont-fix and duplicate) and that it does not touch any issue that is assigned to a user, project or milestone.

I know that the bot is not the perfect answer since it is really annoying if you have an abandoned project and you issue get's closed without anyone looking at it. But I think that is not the case on this project :)

jwillmer commented 3 years ago

I don't know if I should add the new appveyor.yml to this branch since it get's merged into master. And the master appveyor.yml says:

# Note that this YML has significantly diverged from the v2 YML
# when v2 branch comes to be merged to master, there will be
# serious conflicts.  When that happens, you should probably just
# take the YML for v2 and discard what was in master.
craigfowler commented 3 years ago

Agreed, but it implies that "If we have any old issues without labels", then if we haven't managed to triage & label them before the cut-off point (whatever is chosen), then they're going to get auto-closed.

This is why I started the discussion the other day, because I could we we have a lot of unprioritized issues needing triage and figured we should have a strategy for getting out in front of them. Using stale bot is a different strategy and whilst one is not more "correct" than the other, IMO we should collectively choose which is right for the project.

I for one don't think it would take a massive amount of resource to triage & prioritize all of the issues we have at the moment into a backlog. Heck, I'd volunteer to do it. Bulk-closing large numbers of them just because they weren't spoken about recently could easily be a case of "throwing the baby out with the bath water".

jwillmer commented 3 years ago

Another note about the config: After 60 days of inactivity the issue get's a tag stale. Then after some time (daysUntilClose: 7) the issue get's closed if no new activity happened in between. We could run the bot and then list all issues with the label stale and have a quick look that we don't loose valuable tasks.

craigfowler commented 3 years ago

Ah, as for the appveyor.yml changes yes. The v2 branch has appveyor build config set up that's good for v2. The master branch has slightly different build steps and due to the history of the files - see https://github.com/elsa-workflows/elsa-core/issues/664 - I already know it's going to be a car-crash merge conflict when it comes down to merging the v2 branch into master.

That comment is basically saying "when that car crash happens, just discard everything that was in the master version of this file and take the content from the v2 branch".

So rather than merging the v2 appveyor into master (which would break it), just replay that one change from the commit I noted above into the master version. You might not be able to cherry-pick but the diff should make it clear what needs to be changed. That YML fix is the only change in the commit.

craigfowler commented 3 years ago

Another note about the config: After 60 days of inactivity the issue get's a tag stale. Then after some time (don't know exact how much) the issue get's closed. We could run the bot and then list all issues with the label stale and have a quick look that we don't loose valuable tasks.

That's certainly interesting. That eases some of my concerns. I'm going to have to run off and get on with my day job work for now though. I'll pick this up again in ~7 hours.

jwillmer commented 3 years ago

@craigfowler can you fix the appveyor script? I merged the changes you requested but it did not work.

craigfowler commented 3 years ago

@jwillmer - you missed off a close-brace at the end of the first block of Powershell ;)

craigfowler commented 3 years ago

So, now I've got a bit more time to stop and think this through, I do agree that it could be useful, but mainly for issues which are requests for support/help. I personally don't think it's so useful for bug reports (ones which we have confirmed are real, at least) and feature enhancement requests.

Because I was wearing "my day-job head" when I looked this morning, I was thinking of an issue tracker as only being a product backlog containing bugs & enhancements. At the day-job we have a totally separate helpdesk who deal with support requests, so I tend to forget they exist. In the Github & open source world, yes, the issue tracker ends up containing a lot of "please help" requests too. For those, I totally see the value of something like stale-bot.

Stale support requests could be auto-closed

For a support-request, if someone asks for help and nobody was able to answer it after a few weeks (or perhaps even a month) then actually there's a really high chance that nobody will ever be able to answer it.

This is totally worthwhile. It stops the issue tracker from filling up with unanswered questions which we're ignoring anyway. If the person who asked the question urgently needs an answer they can bump with a comment I guess. It's reasonable to accept that sometimes people will ask questions that we collectively can't answer, especially because we're all volunteers.

Stale change-requests are "do later"

On the other hand, change requests (confirmed bugs & enhancements) which aren't high-priority enough to be scheduled now can be given the label do later. They become our product backlog and we don't want to close them unless we think that they should be wontfix.

As we develop, we're going to be finishing releases. As that happens, we take a section away from our roadmap and turn it into a changelog. When we do that, we can add a new section to the end of the roadmap with the next batch of things we would like to implement. That's when we can skim through the do later issues to see if there's any that we'd like. We can take the ones which are best/highest-priority, discuss & agree any details we deferred & promote them to do soon, ready for scheduling & implementation.

Where it comes to looking for issues to work on, we can totally use a filter -label:"do later" so that we are not encumbered by them.

So, actually, I think I'm trying to agree with you!

So, having thought about all of this, what I'm really saying is that I think it's a good idea. The only refinement I'd suggest is that we have stale-bot work on:

@sfmskywalker ^^ Do you have any thoughts on any of this (including the earlier discussion)?

jwillmer commented 3 years ago

Just one note: Any new label we create we need to exclude for the bot to process like I did in this commit - there is no option for "only process no label and labels in this list"

craigfowler commented 3 years ago

Ah, OK. That's not the end of the world for now though ;)

I'll push in a commit to deal with the new/edited labels I recently changed. Update: Done!

jwillmer commented 3 years ago

@craigfowler prio high and low where meant to be used in the projects to define the priority of tasks. I know you can just sort them but often you like to mark high prio tasks so that you have an easy way of seeing that all prio tasks are at the top of the list.

craigfowler commented 3 years ago

@jwillmer ah, for the priority labelling thing I've started #702 . Again, there was a parallel discussion about that which you might not have seen. That happened as #688.

In short what I did was swap them for do now/do soon/do later. High & low priority were edited to become soon & later. "do now" is a new label which currently has no issues (and ideally it's very rarely used).

But - I shouldn't have just edited the labels. I had a feeling that maybe you hadn't seen the discussion at #688 when you set the priority labels up.

craigfowler commented 3 years ago

Generally-speaking btw, if I've got something which I'm suggesting project-wide, I'll post it in that design & tech category first.

sfmskywalker commented 3 years ago

Awesome stuff guys. Let's start with this and see if it works for us.