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

BC: Do not run stale logic for newly stale objects #1075

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jsoref
Copy link

@jsoref jsoref commented Aug 11, 2023

Description:

Simplify code for handling the transition from not-stale to stale. Do not run lots of code that makes no sense during this transition.

https://github.com/jsoref/stale-bot-debug/actions/runs/5828506506/job/15806335430#step:2:97

BEHAVIOR CHANGE:
This change will force action users to trigger two action runs if they want to both mark an item as stale and close it.

(You could technically write a workflow that called the action twice, it doesn't actually require two workflows.)

Related issue:
Add link to the related issue.

Check list:

  • Mark if documentation changes are required. -- Possibly. It depends on how good the documentation is. Based on how I encounter this action used in the real world (most projects are misconfigured and really don't understand what the actions does or how it works and certainly not how to achieve what they want), my bet is the documentation is sufficiently poor that it doesn't really, beyond a release note that this is a behavior change.
  • Mark if tests were added or updated to cover the changes.

https://github.com/jsoref/stale-bot-debug/actions/runs/5828506506/job/15806335430#step:2:97

::group::[actions#2] Issue actions#2
[actions#2] Issue actions#2
  [actions#2] Found this issue last updated at: 2023-08-09T14:33:12Z
...
  [actions#2] Marking this issue as stale
  [actions#2] This issue is now stale
  [actions#2] This issue is already stale

At this point, things are already pretty bad. The issue wasn't _already_
stale, it was _just marked_ stale. There was a lot of code trying to
keep this state in mind, and it yields some really lousy outcomes...

  [actions#2] Checking for label on this issue
  [actions#2] Issue marked stale on: 2023-08-11T03:11:21Z
  [actions#2] Checking for comments on issue since: 2023-08-11T03:11:21Z

Great, so, while the workflow is running, it's looking to see if any new
comments have arrived since the workflow itself marked the issue as
stale. The odds of there being any are very close to 0, and really
there's no point in checking now, it makes much more sense to check the
_next_ time the workflow runs.

  [actions#2] Comments that are not the stale comment or another bot: 0
  [actions#2] Issue has been commented on: false
  [actions#2] Days before issue close: 3
  [actions#2] The option remove-stale-when-updated (​https://github.com/actions/stale#remove-stale-when-updated​) is: true
  [actions#2] The stale label should not be removed
  [actions#2] marked stale this run, so don't check for updates

Here we finally think about the fact that we're in this edge case.

The only thing that makes sense to keep, and which is moved by this
change is:

  [actions#2] Removing all the labels specified via the labels-to-remove-when-stale (​https://github.com/actions/stale#labels-to-remove-when-stale​) option.
  [actions#2] Removing the label "label-to-add-when-unstale" from this issue...
  Error: [actions#2] Error when removing the label: "Label does not exist"

We return to nonsensical tasks:

  [actions#2] Issue has been updated since it was marked stale: false
  [actions#2] Issue has been updated in the last 3 days: true
  [actions#2] Stale issue is not old enough to close yet (hasComments? false, hasUpdate? true)

Note that it's a really rude behavior to mark an item as stale and close
it in the same action.

BEHAVIOR CHANGE:
This change will force action users to trigger two workflow runs if they
want to retain that rude behavior.

  [actions#2] 5 operations consumed for this issue
  ::endgroup::
@jsoref jsoref requested a review from a team as a code owner August 11, 2023 14:46
@dsame dsame self-assigned this Aug 14, 2023
@dsame
Copy link
Contributor

dsame commented Aug 14, 2023

Hello @jsoref , thank you for the PR, it seems to be reasonable, we need some time to investigate it

@dsame dsame added the investigation The issue is under investigation label Aug 14, 2023
@dsame dsame removed their assignment Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigation The issue is under investigation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants