Skip to content

Conversation

@Snailedlt
Copy link
Collaborator

@Snailedlt Snailedlt commented Oct 31, 2022

Double check these details before you open a PR

  • PR does not match another non-stale PR currently opened

Features

This PR closes NONE

Didn't create an issue for this since it was quicker to just implement it

Notes

You can now run the check-bot by adding the bot:check label to a PR. This is powerful, since it lets us get new PR info (like the title) as input to check-bot without creating a new PR. So instead of creating a new PR everytime we fix something, we can instead re-add the bot:check label to re-run it with new parameters :)

PS: https://github.com/devicons/devicon/wiki/Our-automated-tasks-and-bots#check-bot needs to be updated with info on how to run the check bot after this PR is merged into master

You can now run the check-bot by labeling a PR with 'feature:icon'
@Snailedlt Snailedlt requested a review from Panquesito7 October 31, 2022 16:08
@Snailedlt Snailedlt added enhancement devops Devops/automation related enhancements labels Oct 31, 2022
@Snailedlt Snailedlt changed the base branch from master to develop October 31, 2022 16:08
@Snailedlt Snailedlt added the hacktoberfest-accepted Accepted to be counted towards Hacktoberfest label Oct 31, 2022
@lunatic-fox lunatic-fox self-requested a review November 19, 2022 11:19
lunatic-fox
lunatic-fox previously approved these changes Nov 19, 2022
Copy link
Contributor

@lunatic-fox lunatic-fox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loved it! 🤩
To me it's approved! ✔

@Snailedlt
Copy link
Collaborator Author

@lunatic-fox this is actually a PR I forgot to edit 😅
I wanna change it to a different label, since we use this label to label any icon PR, so the workflow would run twice on each PR. Instead I wanna use a new label (maybe bot:check?) to run the workflow. That way we can more easily choose when to run the PR without having to remove the feature:icon label 😁

@lunatic-fox lunatic-fox dismissed their stale review November 19, 2022 11:51

Work in progress.

@lunatic-fox
Copy link
Contributor

Hmm... I think the workflow needs to trigger in two events:

  • When the pull request is made.
  • When a label is created.

Then if the "created" label is, for instance, bot:check it'll run the proper steps. Otherwise it should skip the job.

@Snailedlt
Copy link
Collaborator Author

@lunatic-fox I changed the label... ready for review again now :)

Hmm... I think the workflow needs to trigger in two events:

  • When the pull request is made.
  • When a label is created.

Then if the "created" label is, for instance, bot:check it'll run the proper steps. Otherwise it should skip the job.

It already automatically runs on new PR's, provided that the PR title starts with new icon or update icon, so this PR only adds the additional trigger when the bot:check label is added to the PR :)

@Snailedlt Snailedlt changed the title Feature/run check bot on labeling pr with feature icon Run check bot on labeling PR with bot:check Nov 19, 2022
@lunatic-fox
Copy link
Contributor

lunatic-fox commented Dec 12, 2022

I tested it in a private repo, simulating the #1558 PR with the proper label bot:check. However, it seems that there is some kind of priority in "Peek Icons" workflow.

What happens

  • Made a new icon PR

    • check_icon_pr.yml workflow starts to run.
    • ✔ Then, post_check_icon_pr_comment.yml runs.
  • Add bot:check label

    • 🚫 peek_icons.yml workflow is skipped.
    • 🚫 Then, post_peek_screenshot.yml is skipped.

My guess is since "Peek Icons" is more specific, because it's label triggered, runs first than "Check Icon PR", locking the target workflow.
I think it's easy to fix it. 🙂

Panquesito7
Panquesito7 previously approved these changes Feb 8, 2023
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 🚀 apart from what @lunatic-fox mentioned.

@Snailedlt
Copy link
Collaborator Author

@lunatic-fox I didn't understand that explanation fully, but if it's an easy fix, feel free to fix it. If you want me to fix it I'm afraid I'll need to be spoon fed the explanation again :S

@Panquesito7
Copy link
Member

Ping.

@Snailedlt
Copy link
Collaborator Author

@lunatic-fox You mentioned an easy fix here, but I'm not sure what you mean the issue is. Do you mind fixing it?

@Snailedlt
Copy link
Collaborator Author

or should I just merge it as-is?

@canaleal
Copy link
Member

Has not been updated in a while, closing, but it can be reopened.

@canaleal canaleal closed this Nov 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devops Devops/automation related enhancements enhancement hacktoberfest-accepted Accepted to be counted towards Hacktoberfest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants