workflows/stale: fix workflow syntax

[Rylan12]

• Have you followed the guidelines for contributing?
• Have you checked that there aren't other open pull requests for the same formula update/change?
• Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
• Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
• Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

---

The stale action that I added yesterday failed with a message saying:

Invalid workflow file:

The workflow is not valid. .github/workflows/stale.yml (Line: 22, Col: 13): A sequence was not expected,.github/workflows/stale.yml (Line: 26, Col: 13): A sequence was not expected

You can see the failed workflow run here

This PR (hopefully) fixes the syntax to match the examples given in the upstream repo (not sure why I didn't try to match them for the first round...)

For testing purposes, is it worth adding a workflow_dispatch trigger to allow manual testing? I don't think that is something we need to be able to do once the action seems to be working, but may be helpful if we continue to have problems. I'll hold off for now, but figured I'd through the idea out there.

CC: @jonchang

[jonchang]

For testing purposes, is it worth adding a workflow_dispatch trigger to allow manual testing?

Might as well. It's not like with repository_dispatch where we can realistically only have one per repository

[MikeMcQuaid]

For testing purposes, is it worth adding a workflow_dispatch trigger to allow manual testing? I don't think that is something we need to be able to do once the action seems to be working, but may be helpful if we continue to have problems. I'll hold off for now, but figured I'd through the idea out there.

Instead I'd suggest making it run on push events that change this workflow file. Then you can push to a homebrew-core branch instead of a fork and have it run.

[Rylan12]

A few questions (due to me being very new to GitHub actions):

Might as well. It's not like with repository_dispatch where we can realistically only have one per repository

Sorry, not familiar enough with repository_dispatch to really understand what you mean here. Based on these docs it looks like is used to trigger an action from a post request. Is the difference between repository_dispatch and workflow_dispatch that the latter can only be triggered manually via the GitHub UI? Either way sounds like you're indicating that repository_dispatch isn't the way to go.

---

Instead I'd suggest making it run on push events that change this workflow file. Then you can push to a homebrew-core branch instead of a fork and have it run.

Okay, I think I understand what you mean. You're suggesting that I change to this?

on:
  push:
  schedule:
    # Once every day
    - cron: "0 0 * * *"

You did say:

push events that change this workflow file

I'm not sure how to restrict to events that change the workflow file. Like this, maybe?

on:
  push:
    paths:
      - .github/workflows/stale.yml
  schedule:
    # Once every day
    - cron: "0 0 * * *"

Then you can push to a homebrew-core branch instead of a fork and have it run.

Sorry, I'm not sure what you mean here.

[SMillerDev]

I'm not sure how to restrict to events that change the workflow file. Like this, maybe?

That looks like what we want.

Then you can push to a homebrew-core branch instead of a fork and have it run.

It probably doesn't run from a fork, so if you want to test you can git push origin HEAD:stale_test and it'll run (because there is a push in core with that file)

[Rylan12]

That looks like what we want.

👍 Thanks!

It probably doesn't run from a fork, so if you want to test you can git push origin HEAD:stale_test and it'll run (because there is a push in core with that file)

Oh, I see. You're saying that it's not running because this PR is from a fork, right? If I had instead opened this PR from another branch in the main repo it would have run? So pushing to the stale_test branch is basically just creating a temporary branch that will cause the workflow to run (because it's a push). Then, once this PR is merged I can just delete that branch. Is that what you're saying?

[SMillerDev]

Oh, I see. You're saying that it's not running because this PR is from a fork, right?

Yes

If I had instead opened this PR from another branch in the main repo it would have run?

It would with that config

So pushing to the stale_test branch is basically just creating a temporary branch that will cause the workflow to run (because it's a push). Then, once this PR is merged I can just delete that branch. Is that what you're saying?

Yeah, that sounds about right

[Rylan12]

Thanks! I'll give that a try

[Rylan12]

Here's the workflow run.

It looks like that worked for issues. Here are a few issues that were marked stale: #59802, #62084. Here's an issue that was closed: #64167. It also looks like it stayed away from in progress and help wanted issues (as expected)

It didn't run on any PRs. Looking at the logs, it found stale PRs but didn't mark them saying Skipping pr due to empty stale message. I forgot we need to separately define the stale-issue-message and stale-pr-message, so I'll make that change here next.

Looking good!

[Rylan12]

Okay, looks like this works for PRs now!

Here are a few PRs that were marked as stale: #63160, #63161. Here are some PRs that were closed: #64698, #64699.

While this is great, I was expecting it to close/mark a few more PRs. For example, #61838 doesn't seem like it's had any activity for over a month but it wasn't marked. Maybe there's a limit to how many can be marked in one go?

Edit: this shows up in the logs:

Warning: Reached max number of operations to process. Exiting.

From the action docs, it looks like the operations-per-run option can be set. It defaults to 30. Not sure if we want to modify this or not. I think most days we won't have even close to 30 operations, so I'd say no need to change it and we can just run it a few times right now to get through the queue of old PRs.

Edit 2: Yep, just manually ran again and more were marked (and we reached the limit again). I'll bump the limit for these initial runs but won't modify it in this PR.

[MikeMcQuaid]

Looks great, nice work @Rylan12. If it'd be possible could you add this to Homebrew/brew after this is merged here?

[MikeMcQuaid]

(similarly if any of these others seem not to be working it'd be good to investigate and replace with actions?)

[MikeMcQuaid]

https://github.com/Homebrew/.github/tree/master/.github

[Rylan12]

One final thing to keep in mind:

This action is supposed to remove the stale label if there's a comment from a user who is not the "actor" (does that mean author? not sure...). However, if this is run only once a day the label won't be automatically removed until the next run (which is the next day). I think that's fine, right?

[Rylan12]

If it'd be possible could you add this to Homebrew/brew after this is merged here?

Absolutely. Just wanted to try it in one repo at a time.

(similarly if any of these others seem not to be working it'd be good to investigate and replace with actions?)

I've glanced at the others. We can probably adapt the stale action to the no-response workflow. The others will probably need a different action. I can do some research to see if anything looks right.

Should/is there a way to have these configurations live in Homebrew/.github instead of in each individual repo? Not sure if it saves us any work. They won't automatically run anyway.

[MikeMcQuaid]

I think that's fine, right?

Yup, seems to be!

I've glanced at the others. We can probably adapt the stale action to the no-response workflow. The others will probably need a different action. I can do some research to see if anything looks right.

Perfect ❤️

Should/is there a way to have these configurations live in Homebrew/.github instead of in each individual repo? Not sure if it saves us any work. They won't automatically run anyway.

I don't think so, unfortunately 😭

[Rylan12]

Alright, sounds good. I'll merge this. I'm currently in the middle of migrating the audit exceptions so once I'm done with that (or at least at a stopping point) I'll add the stale workflow to brew too (and look into the others).

No reason to keep those old config files in Homebrew/.github anymore (once they've been replaced), right?

[MikeMcQuaid]

No reason to keep those old config files in Homebrew/.github anymore (once they've been replaced), right?

Agreed!

[jonchang]

This action is supposed to remove the stale label if there's a comment from a user who is not the "actor" (does that mean author? not sure...).

I believe this means that the stale label won't be removed if BrewTestBot comments.

[Rylan12]

Possibly. That would certainly make sense. I checked the source code and it looks like it excludes comments by github_1.context.actor (not sure what github_1 is exactly...)

In #63176, though, a user commented after the stale label was applied. In the most recent run (which was after the comment was posted), the action logged the following for the issue:

Found issue: issue #63176 last updated 2020-11-26T16:32:07Z (is pr? false)
Found a stale issue
Checking for label on issue #63176
Issue #63176 marked stale on: 2020-11-26T16:32:07Z
Checking for comments on issue #63176 since 2020-11-26T16:32:07Z
Comments not made by actor or another bot: 0
Issue #63176 has been commented on: false
Issue #63176 has been updated: true
Stale issue is not old enough to close yet (hasComments? false, hasUpdate? true)

This is surprising to me because:

1. There was a comment made
2. Not sure what hasUpdate means (I assumed it meant changes were pushed for PRs but not sure about for issues). Regardless, if it's true, shouldn't the action remove the stale label?

Edit: I found another example where the stale label is removed (as expected): #62722 (comment)

Edit 2: I looked into it a little more. Based on actions/stale#73 (comment), actions/stale#192, and actions/stale#21 (comment), it seems that the "actor" in this case is the last person to modify the workflow file i.e. me. That doesn't explain it but is... interesting