[Maintenance] github actions - "Ready for review" labels

[MatissJanis]

We are currently using trafico to label PRs with "WIP" and "Ready for review" labels.

There are a few problems with our current solution that cannot be solved with trafico:

1. draft PRs get the "ready for review" label (they should not get this label)
2. PRs with failing CI/CD checks also get "ready for review" label (ideally they should not)

The goal of this maintenance task is to solve the above two issues. It might require picking a different tool than trafico. Or alternatively: to fork trafico and implement the missing functionality.

[subnut]

1. draft PRs get the "ready for review" label (they should not get this label)

Should they also get the default "WIP" label, or a new "Draft" level?
I prefer the former.

2. PRs with failing CI/CD checks also get "ready for review" label (ideally they should not)

Should they get the "WIP" label too? Or should they have a new label? (eg. ❌ Failing CI)
I prefer the latter.

[MatissJanis]

More granularity would definitely be better. But if that's not possible or hard to achieve, then re-using the existing labels is also fine.

[carkom]

Can we just create a workflow to add [WIP] to the begining of every new PR title? At that point trafico would pick it up and label it. Then the expectation is that the owner of the PR would remove the WIP when it's ready for review.

I like the idea of a new "Failing CI" label. I think trafico is working well, might be good to add the functionality to it. I'm saying that with no expertise on how difficult that would be. 🤷

[MatissJanis]

That would work too if you want to give it a go. It wouldn't solve the issue fully though.. but still worthwhile doing in the short-term.

[matt-fidd]

With the trafico bot currently down, I've been playing around with probot and it doesn't look hard to create a bespoke bot for actual.

Is this something what would be welcomed?

I've got one running currently that replicates the WIP functionality (and even adds the [WIP] prefix on for newly opened PRs removing the need for the github action). It's completely TS and unit-testable too which is a bonus

[MatissJanis]

Cc @twk3 (see comment above)

[twk3]

@matt-fidd do you think the logic you've written is something that could be moved to an action (using actions/github nodejs package rather than probot)? Actions are easier for others to customise and use without needing additional hosting.

I'm attempting the same thing with a fork of trafico (moving it to an action I mean), but was going to leave it rough (JS, no unit tests) with the anticipation that we would rewrite something eventually. Sounds like you've already got that part going! If it's something you're willing to publish as open source, we'd love to take a look.

[matt-fidd]

@twk3 It's definitely worth a look! I can't say that I've used actions before but I'd be willing to have a look at implementing if that's something you'd be looking for?

I've pushed the bot as-is up to https://github.com/matt-fidd/actual-bot/ for your viewing pleasure (or displeasure really as this is my first attempt at typescript). It's very rough at the moment but if the bot is something you'd be interested in then it could easily be expanded to replace trafico, add add the functionality that Matiss requested originally in this issue.

If you'd definitely like to go in the action direction instead then let me know and I can have a look!

[psybers]

You can make a workflow that triggers after the suite finishes (with help from ChatGPT):

on:
  check_suite:
    types: 
      - completed

jobs:
  check_failure:
    if: github.event.check_suite.conclusion == 'failure'
    runs-on: ubuntu-latest
    steps:
      - name: Print a message
        run: echo "A check suite has failed on this pull request."

And then I'm sure there is a way to add a label to a PR from a workflow. ChatGPT suggested a script, which would work, but then you would need to store an API token somewhere.

[psybers]

I guess that is what GitHub recommends too: https://docs.github.com/en/actions/managing-issues-and-pull-requests/adding-labels-to-issues

[matt-fidd]

I've had a go at some github workflows, I'm sure they're not perfect but they do at least work!
https://github.com/matt-fidd/bot-test/tree/master/.github/workflows

They're much slower to act then the bot and result in a status check appearing at the bottom of every pull request - not sure how to avoid this

[psybers]

@matt-fidd Could you open a PR with those workflows against this repo so we can move the discussion there?

[matt-fidd]

@matt-fidd Could you open a PR with those workflows against this repo so we can move the discussion there?

I'll push this up to a WIP PR for you now and write the rest of the suite

[matt-fidd]

Up for you #2900 @psybers

[matt-fidd]

There's a thread going in #development on discord with me and twk3 but just thought I'd drop it here too. We came to the conclusion that github actions weren't the way forward because we'd lose the functionality of the approved and changes requested labels. @twk3 settled on a github bot as a serverless function through Netlify.

Finally had the time tonight to get it over the line, it now replicates the existing functionality with trafico, as well as extending to

• handle drafts
• add a failing CI status

as requested above

If you get a chance to have a look I'd welcome any feedback

Repo link
https://github.com/matt-fidd/actual-bot

Installation link (hosting on my server for now, will keep it online for anyone to test)
https://github.com/settings/installations/52299031

No test suite as yet, I'm struggling with the framework that probot comes with, if anyone has any pointers I'd appreciate it!

[youngcw]

Can this be closed?

[MatissJanis]

2. PRs with failing CI/CD checks also get "ready for review" label (ideally they should not)

This is not yet done.

[matt-fidd]

2. PRs with failing CI/CD checks also get "ready for review" label (ideally they should not)

This is not yet done.

The label for Failing CI is built into the new bot, but it was disabled at the request of @twk3. It works in testing but I believe tw3k saw some weirdness that I couldn't reproduce.

Maybe we could enable it on actual-server first to let me iron out any possible kinks before rolling to actual?

[twk3]

@matt-fidd the path where it adds ready for review doesn't take into account the other factors like review status before it marks a PR as ready for review. And the presence of the failingCI label I don't think prevents the other events from adding ready for review.

[MatissJanis]

I think we can close this issue as complete. Thoughts?