Skip to content

Conversation

@Thomas-Boi
Copy link
Member

Hello guys,

This is the second part of the repo upgrade. This branch will be focusing on creating the new workflow as shown in this announcement.

The branch will be tested a couple times before it's ready to be merge. Please don't merge until I leave a comment saying that it's functional.

Cheers,

@Thomas-Boi
Copy link
Member Author

@amacado there's an issue with the "bot: build" label. When I check for it inside the build_icons.yml, GitHub complains that there's an error.

image

Turns out that the ":" mess up the syntax, which means we have to rename the build label. Here are some variations that I tested that seems to work:
-bot:build with no space in between
-bot-build with a "-"

Let me know what you think. I'll create a bot:build label just for testing purpose

Copy link
Member

@amacado amacado left a comment

Choose a reason for hiding this comment

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

I vote for bot:build as you suggested.
Also renamed our existing labels feature:icon and request:icon to match the syntax. I don't like spaces anyway ;-)

commit_message: Build new icons, icomoon.json and devicon.css
COMMIT_MESSAGE: "Built new icons, icomoon.json and devicon.css"
PR_BRANCH_PREFIX: "build/feature/"
PR_BRANCH_NAME: "test-123"
Copy link
Member

@amacado amacado Oct 5, 2020

Choose a reason for hiding this comment

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

Maybe test-123 could a little more dynamic? ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Haha, sorry about that. I was in the middle of testing the trigger and wasn't sure how to do string interpolations in yml files.

@Thomas-Boi Thomas-Boi added bot:build and removed bot:build request:icon When requesting a new icon to be added to the collection or a refactor use this label in your issue labels Oct 7, 2020
@amacado
Copy link
Member

amacado commented Oct 7, 2020

@Thomas-Boi let me know when you're ready for a new review! 👍🏻

@Thomas-Boi
Copy link
Member Author

Thomas-Boi commented Oct 7, 2020

Hey @amacado,

The workflow file now properly push any changes to a new branch and opens a PR on the triggered branch. This took awhile because I couldn't find an action on the marketplace that works for us.

Features:

  • New trigger that would only run the script when a label is added to the PR
  • A PR action that commits our changes, push a new branch then open a PR onto the triggered branch.
  • PR action also deletes our branch after it's closed or merged.
  • Checks to ensure that the previous steps ran before continue

To test the script:

  • Remove all the labels
  • Add label test123.
    • Check the Actions tab
    • You should see a workflow run but then skipped => This is expected since it's not bot:build
  • Add label bot:build.
    • Check the Actions tab
    • You should see a workflow run.
    • Once it's done, check the Pull Request tab => You should see a new PR
  • Check the PR's file changes to confirm that files have been built (these should be the .eot and .woff files).
  • You can merge them if you want to.

Note:

  • This is the label check inside the script:
    if: contains(github.event.pull_request.labels.*.name, 'bot:build')
  • Unfortunately, this does not check whether the recently added label is named bot:build. Instead, it checks whether the branch already has a bot:build label. I couldn't find an event that would give us only the label that was added.
  • You can see this behaviour if you add another label to the PR. The script would gets trigger even though the label is not bot:build.
  • Here is the pull_request object that I'm checking in the if statement above. Please feel free to give it a look to see if you can find a better property that we can use to check.

What's next:

  • We want to have dynamic naming for the PR branch. To do that, I'd need your help to define the format as shown on Discord. For other contributor's reference, here are my questions:

In our workflow, after we built the fonts we will create a branch and push it
The name will be in this format build/feature/{{id-of-issue}}-{{description}}
Is the {{id-of-issue}} the id of the branch we are PR into or the issue that it'd solve?
And what is the {{description}}? Is it the commit message of the branch that the user push?

  • We can also customize the PR's body. Currently, it is very vague and I think the owner of the branch would like some more details on what's being introduced into their branch.

Feel free to leave your comments or mess with the build file :). If you have any questions, I'll happily answer it.

Copy link
Member

@amacado amacado left a comment

Choose a reason for hiding this comment

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

@Thomas-Boi hello! Once again: Excellent work, happy to approve this pr! Been curious to test it on the pending pull requests #283, #284, #296, #297. Maybe you are willing to check one or two of those (or all ;-)) and try our updated workflow?

in my short test the delete-branch: true parameter did not work, i had to delete the branch manually, we should have an eye on it and see if this error stays or was just temporary.

@amacado amacado linked an issue Oct 9, 2020 that may be closed by this pull request
@Thomas-Boi Thomas-Boi merged commit 8f863d6 into develop Oct 9, 2020
@Thomas-Boi Thomas-Boi deleted the upgrade-workflow branch October 9, 2020 16:32
tariq86 added a commit to tariq86/devicon that referenced this pull request Oct 10, 2020
GCHQDeveloper926 pushed a commit to GCHQDeveloper926/devicon that referenced this pull request Dec 20, 2024
Changed build_icons.yml to trigger on label added and create new pr
GCHQDeveloper926 pushed a commit to GCHQDeveloper926/devicon that referenced this pull request Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

On the new workflow and pull request standards

3 participants