-
Notifications
You must be signed in to change notification settings - Fork 111
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
Verify PR labels #1435
Verify PR labels #1435
Conversation
.github/workflows/require_label.yml
Outdated
with: | ||
mode: minimum | ||
count: 1 | ||
labels: "documentation, bug, bugfix, chore, dependencies, enhancement, feature, gh-actions, ignore-for-release, security, sorbet" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took this list from all the labels available, and removed the ones that seem irrelevant for the release notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2b8267c
to
cba080e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can make it add a comment to the PR that will explain when and why it is failing: https://github.com/marketplace/actions/require-labels#customising-the-failure-message--comment
Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we want to add a add_comment: true
option, and possibly a message
option, to explain why the PR can't be merged?
We can only use public actions if the maintainer owner is a 'verified creator': mheap/github-action-required-labels#48 I'll give it a few days but if that doesn't work out then I suppose we could fork it? |
Unfortunately this seems like a no-go: |
Motivation
Closes Verify PR labels before merging
Implementation
There are a few other actions very similar to this, but this one seems to have the features we need, so I'm trying out.
Thoughts: