Label required for merging instead of part of CI

[dschult]

We've had a couple of confused first PR submitters who think the red-x of not having a label means that they did something wrong. It might be better to make having a label be a requirement for merging (since it is assigned by the maintainer who presumably will merge it) rather than a Github-Action checking whether there is a label.

How hard would it be to configure the label check as a merge requirement rather than a CI action?

[lagru]

Thanks for the suggestion. I get why this can be a confusing contributor experience. I'm not sure how easy it is to throw a wrench in the merge process but that would be ideal.

Maybe we could use a submitted review (pull_request_review, submitted) as a trigger for the job? For most projects that's bound to happen before merge and would likely come from someone that wouldn't be confused by the failure?

[bsipocz]

I really don't like PR/issue templates, but having different, label setting templates for the users to choose may remove that confusion.
Then the maintainers can correct if a PR was miscategorised.

[bsipocz]

But triggering on review (both submitted, but also edited and dismissed) sounds like an excellent solution.

[stefanv]

I just looked into this a bit, and currently GH has very specific behaviors to choose from when it comes to merge blocking. As such, the only places to inject this (not as a standard check) would be in a merge queue, or on the main branch, after merge.

@lagru's suggestion may work: we only run the label check once the PR has been approved: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-a-workflow-when-a-pull-request-is-approved

I don't know if this will sufficiently block merging, but it's worth a try.

[stefanv]

This does seem to do the trick: it only runs the label checker after the first review is made:

https://github.com/stefanv/test-labeler/blob/main/.github/workflows/label-check.yaml

[jarrodmillman]

This does seem to do the trick: it only runs the label checker after the first review is made:

It also makes every PR fail unless you add the label before the review.

It fails on

  pull_request_review:
    types: [submitted]

and then passes on

  pull_request:
    types: [reopened, labeled, unlabeled]