-
Notifications
You must be signed in to change notification settings - Fork 366
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
Update how stale handles exempt items #874
Conversation
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.
Just some language updates that I feel are better, feel free to pick and choose.
Also, we've talked offline about this but just to put the comments in a PR. I personally don't think we should be supporting #136 further and should revert that change entirely.
There are 2 logical ways to infer what the purpose of this action is;
- The first, in Remove stale label when issue is tagged with exempt label after marking stale #136, is that the inference is that the
stale
label itself is "owned" by this action. With this in mind,exempt
meansexempt from usage of the label
. - The second is that this action "manages" the
stale
label on issue's/pr's. With this in mind,exempt
meansexempt from the action managing that issue/pr
.
Personally I feel that the second inference is more appropriate to the spirit of this action. It also means that we don't have to write edge-cases to actively prevent users from labeling their issue's/pr's, which really leads to a confusing user experience (see #766).
This would mean reverting this PR (which is now adding an additional configuration option to handle edge cases), as well as #136 and being more clear on definitions. I'll leave it to discussion from the rest of the team
So to paraphrase @vanZeben there are two options:
|
ec30f21
to
330059b
Compare
I've updated the PR to be aligned with comments above. Moving forward this will simplify how stale handles exempt items and will allow us to avoid edge cases such as the flag that was previously mentioned in 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.
LGTM
Changes
New option addedremove-stale-from-exempt-items
Context
Originally stale bot would not process exempt items.
In #136, stale bot was updated to remove stale labels from items with exempt labels. This introduced a problem where if a user wanted to manually add a stale label to an item with an exempt label, the
stale
label would automatically be removed.To resolve this, I've added a flagremove-stale-from-exempt-items
which removes thestale
label from an exempt item with recent updates. This comes at the cost of a couple of api calls, so by default this option is set to false. When the flag is disabled stale will continue to not process exempt itemsTLDR: Stale bot removed stale labels from exempt items no matter what. The new optionremove-stale-from-exempt-items
allows stale bot to only remove the stale label from exempt issues when they've been recently updatedThis PR reverts #136 and adds additional documentation and logging in order to clarify the behaviour of exempt items
Resolves #766