Skip to content

Conversation

@lunatic-fox
Copy link
Contributor

Double check these details before you open a PR

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

Features

This feature adds newliner.py and changes optimize_icons.yml to optimizer.yml.

About optimizer

optimizer workflow is triggered by any user commit to the pull request (synchronize) and by feature:icon label.

  • synchronize - This trigger adds a newline at the end of each file that is missing it.
  • feature:icon - This trigger optimizes any icon with SVGO, it also adds a newline at the end of the optimized icon.

This PR closes #1624 and changes #1554

Notes

We can change the label that triggers the icon optimization.

I've decided to make these two actions in one workflow, because some triggers are conflicting or not working properly, but if some way can be found to split them it would be nice to apply it.

Copy link
Collaborator

@Snailedlt Snailedlt left a comment

Choose a reason for hiding this comment

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

I suggest we use newline-action instead of making our own newline script. And also I would prefer it if the script was moved to a different workflow all together.

See comment below for more info

Comment on lines +38 to +44
- name: Setup Python v3.10
uses: actions/setup-python@v4
with:
python-version: '3.10'

- name: Add newline
run: python ./.github/scripts/newliner.py ${{ env.SVGO }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason why we can't use the newline-action?

Suggested change
- name: Setup Python v3.10
uses: actions/setup-python@v4
with:
python-version: '3.10'
- name: Add newline
run: python ./.github/scripts/newliner.py ${{ env.SVGO }}
- name: Add newline
uses: Logerfo/newline-action@0.0.4
with:
github-token: ${{ secrets.GITHUB_TOKEN }}

This would add newline to any file, not just SVG files.
It's also possible to ignore files in a config (see link above)

Come to think of it, maybe it would be better to add the newline in a separate script like this:

name: Add newline
on:
  pull_request:
    types: [synchronize, opened]
jobs:
  build:
    name: newline-action
    runs-on: ubuntu-16.04
    steps:
      - uses: actions/checkout@master
      - uses: Logerfo/newline-action@0.0.4
        with:
          github-token: ${{ secrets.GITHUB_TOKEN }} # The `GITHUB_TOKEN` secret.
          config-path: .github/newline.yml # The path of the addtional configurations file.

Copy link
Member

Choose a reason for hiding this comment

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

This sounds good to me. 👍

@Panquesito7 Panquesito7 added the devops Devops/automation related enhancements label Mar 6, 2023
@lunatic-fox lunatic-fox marked this pull request as draft April 30, 2023 00:02
@canaleal
Copy link
Member

Has not been updated in 2 years. Closing, but 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants