-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat: Add labeler workflow #523
Conversation
The labeler workflow will add the `feature:icon` label when the `icons` folder is being modified. On PR approval, it will add the `approved` label.
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 the "feature:icon" labeler workflow would be a good thing to have. The "approve" label though I'm not so sure. @amacado what do you think?
@@ -0,0 +1,14 @@ | |||
on: pull_request_review |
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'm not sure whether we need the approved label. Usually, once things are approved we just merge it.
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.
If we're going to use the Probot Stale app, then this will be good, as the app won't mark as stale the PRs that have the approved
label. 🙂
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.
First of all: I ❤️ labels! But I'm not a fan of duplicated functionality. GitHub already provides many options to filter by review status:
I don't see an advantage of having a approved
label indicating the same what GitHub already provides. Or am I missing something?
Auto adding the feature:icon
label is a nice enhancement! Maybe someone can take a look at pull request templates? They might allow to achieve the same result (without a extra workflow). With issue templates (which we already use) it's possible to define a label which is added automatically.
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 don't see an advantage of having an
approved
label indicating the same what GitHub already provides
Well, to me it is easier to use the approved
label. Sometimes, we merge PRs, but we don't approve them; still, we can add the approved
label, and it'll be easier to track them (without being approved).
It's your decision if you want to have it in Devicon or not, but to me, it's a good idea. 🙂
I will add the |
Never mind, it seems I can't push to the |
I think amacado needs to take a look at this before we can merge your code. |
No one is allowed to directly push to |
I think maybe we can use the PR templates instead of an Action. This would requires only 1 extra file rather than 2 for the "feature:icon" label. We can also use the template to enforce certain details, such as a link to the technology page or any other notes. Would you be open to changing from a labelling workflow to a PR template? We can work together to decide what the template should have. As for the "approved" label, unfortunately I don't think it's applicable for our repo. amacado and myself tries to only merge changes that we approve so having a label is a bit redundant. Perhaps if we ever need to merge something that we don't approve of, we will add a special label to it so we can review it again. |
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.
As a result of the discussion I propose to close this pull request, delete the branch and start a discussion about a pull request template. @Panquesito7 are you ok with this?
EDIT: The only thing is that if the user doesn't use the PR template, the label won't be applied. Sounds good to me (apart from the above). 🙂 Thanks anyway. |
The labeler workflow will add the
feature:icon
label when theicons
folder is being modified. On PR approval, it will add the
approved
label. 🙂