Add optimize_icons workflow triggered on push
Double-check these details before you open a PR
- [x] PR does not match another non-stale PR currently opened
Features
This PR closes #1715
Notes
This PR replaces the now closed #1718.
In the previous version, we were trying to make it run on the PR itself, but that doesn't seem to be possible, given the following:
- The
ericcornelissen/svgo-action@v3action does not support aworkflow_runtrigger - The GITHUB_TOKEN does not have permission to write when invoked by PRs from public forks
This version simply runs the optimizer after the PR merge (on the corresponding push). I have tested this with a second fork of the repository, and it seems to work as expected. According to the documentation) when SVGO is invoked on push:
In the push context the SVGO Action will optimize all SVGs that have been added or modified in the commit(s) being pushed. Any SVGs that are in the repository but have not been modified in the commit(s) will not be optimized.
Doing this on the push has the disadvantage of not allowing us to review the optimized versions, but it will ensure all icons in develop are optimized.
Not sure if the commit will be able to be pushed as the develop branch is protected.
Would a PR towards the develop branch work in this case? 🤔
Not sure if the commit will be able to be pushed as the
developbranch is protected. Would a PR towards thedevelopbranch work in this case? thinking
I think branch restrictions can have exceptions for certain users. Maybe we can give the bot or github tokdn permission to push directly? Not sure if that's something we'd want though. I'd prefer if it was done before we merge the PR instead.
@ConX the 2nd issue should be fixable by making a preflight script similar to what we do with the in-develop-labeler script. As for the first issue... Are there any of the other triggers that might work?
Probably we should configure the action to create a PR towards the develop branch only.
Granting access to GitHub Actions isn't really safe. If someone modifies the actions in a PR, they could end up doing unwanted changes to the repository, AFAIK.
Granting access to GitHub Actions isn't really safe.
ACK ;)
If someone modifies the actions in a PR, they could end up doing unwanted changes to the repository, AFAIK.
Well, the action modification must pass a review and merge first. Changing the action in a PR runs with the actions of the current (not the PR'ed) version. Hmm, is that easy to understand? A PR runs with the current actions, even if it wants to introduce changes with the actions. The changed actions are only used after they have been pulled.
In my opinion that makes it ok-ish to have actions with write permissions, just everyone who can merge must know this and watch out for unwanted changes :grimacing: