Skip to content

Revisit policy of auto-closing issues #6866

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.

Sign up for GitHub

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

Jump to bottom
Closed
mhucka opened this issue Dec 20, 2024 · 4 comments
Closed

Revisit policy of auto-closing issues #6866

mhucka opened this issue Dec 20, 2024 · 4 comments
Assignees
@mhucka
Labels
area/github kind/health For CI/testing/release process/refactoring/technical debt items triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@mhucka
Copy link
Contributor

mhucka commented Dec 20, 2024 •
edited
Loading

Currently, the workflow for stale issues (.github/workflows/stale.yml) is set to label something as stale after 30 days of inactivity and auto-close the issue 30 days after that. This is a pretty short duration, so I'd like to propose we review this policy.

Closing inactive issues is a way to reduce the burden on maintainers, of course, but IMHO it's important to balance that against antagonizing contributors. Legit issues don't always see activity within 30 days, and may not be addressable in 60. As it is, we're finding ourselves manually having to reopen some issues, which is actually its own maintenance burden.

Some points to discuss:

  • Duration. longer may be better, but infinite duration is also undesirable; we need a happy middle ground.
  • Issue types. Maybe some types of issues should have different durations. For example, feature requests could potentially have longer durations.
  • Additional procedures. For example, maybe we could add a step of reviewing stale issues & PRs as part of the regular triage process (in biweekly Cirq Cynqs), to decide which ones to deliberately label as triage/wont-fix or similar.

Additional reading:
@mhucka mhucka added the kind/health For CI/testing/release process/refactoring/technical debt items label Dec 20, 2024
@mhucka mhucka self-assigned this Dec 20, 2024
@mhucka mhucka mentioned this issue Jan 6, 2025
Merged
@mhucka
Copy link
Contributor Author

mhucka commented Jan 8, 2025

Some examples of what other projects do:

@mhucka
Copy link
Contributor Author

mhucka commented Jan 8, 2025 •
edited
Loading

The docs for the actions/stale action we use mentions an additional flag that might be worth exploring:

exempt-all-milestones: If set to true, issues or pull requests with a milestone will not be marked as stale automatically.

It kind of makes sense that something assigned to a milestone shouldn't be flagged as stale. If we assigned it to a milestone, then presumably we decided it's something we want done, even if it takes longer than the stale issue timeout. What do people think about using this flag?

@NoureldinYosri NoureldinYosri added the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Jan 22, 2025
@mhucka
Copy link
Contributor Author

mhucka commented Feb 5, 2025 •
edited
Loading

Discussed in 2025-02-05 Cirq Cynq: keep stale bot, continue closure behavior (with extended time longer than 30 days), and expand the comment that the bot leaves upon closure. Have the message say the PR was closed automatically due to inactivity, and if the person wants to reopen it they can.

@mhucka mhucka added triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on area/github and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Feb 5, 2025
mhucka added a commit to mhucka/Cirq that referenced this issue Feb 8, 2025
@mhucka
Fix labels & update messages
96a907d 
1. The previous settings were incorrect. The value of
   `stale-issue-label` should be `status/going-stale`; it is the value
   of `close-issue-label` that should be `status/stale`.

2. Added a couple of labels that would exempt PRs from staleness.

3. Per discussion on issue quantumlib#6866, the message should be more clear
   that users can easily reopen the issue or PR by commenting on it.
@mhucka mhucka mentioned this issue Feb 8, 2025
Merged
@mhucka
Copy link
Contributor Author

mhucka commented Feb 25, 2025

Resolved by PR #7045.

@mhucka mhucka closed this as completed Feb 25, 2025
github-merge-queue bot pushed a commit that referenced this issue Feb 26, 2025
@mhucka
Update & improve workflow for stale issues & PRs (#7045)
2e5c8a2 
* Fix labels & update messages

1. The previous settings were incorrect. The value of
   `stale-issue-label` should be `status/going-stale`; it is the value
   of `close-issue-label` that should be `status/stale`.

2. Added a couple of labels that would exempt PRs from staleness.

3. Per discussion on issue #6866, the message should be more clear
   that users can easily reopen the issue or PR by commenting on it.

* Fix text per review comments

This makes corrections to the "close" messages per
[review comments by @NoureldinYosri](#7045 (comment)).

* Use variables to hold important values

To avoid writing actual duration values in multiple places (and thus
face the risk of not updating some of them if we decide to change
values in the future), this now uses environment variables to store
the important values.

This also provides an easier way of writing the lists of exempt
labels. The stale handler action needs them as a comma-separated
string, and this is a bit of a PITA to edit when we make changes. We
can make it easier on ourselves in the future by using a list format
instead.

* Use a single label (status/stale) for all cases

Per [discussion in
review](#7045 (comment)),
we decided we don't need separate labels `status/going-stale` and
`status/stale`. When an issue or PR is closed due to staleness, the
comment left by the workflow will make it clear why it was closed. We
can simplify things by using a single label.

* Change workflow name to be more like others

More of the workflow names in Cirq are, well, names, rather than
statements about what they do.

* Add workflow & job permissions, per best practices

* Skip the concurrency block to simplify this

Per comments on review, this workflow is not really in need of
guarding against concurrent execution. We can simplify it by removing
unnecessary bits like this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mhucka mhucka

Labels
area/github kind/health For CI/testing/release process/refactoring/technical debt items triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mhucka @NoureldinYosri