calibreapp
[Feature Request] Option to disable new behavior for scanning PR
What problem would this feature solve?
Right now, there is a problem where, in my workflow, I first convert any submitted images in a PR from PNG/JPG to WEBP, and then I run image-actions to compress the WEBP. However, due to the new changed image logic in #339, this collects the initial PNG/JPG files and does not recognize the currently staged WEBPs inside the action.
Describe the solution you’d like to see
I would like to see at least a simple fix where we can disable this behavior. It seems like if you return null in getChangedImages, it will fallback to the whole repo scan behavior which is fine for me for now. Ideally, we could collect the current git status within the action, and diff that against the PR changes to produce an up to date set of changes, but this is more complex.
Are you willing to work on implementing this solution?
Yes
Describe alternatives you’ve considered
No response
Additional context
No response
Code of Conduct
- [x] I agree to follow this project’s Code of Conduct
@mastercoms Thanks for filing.
I am not quite sure I understand - are you removing the PNGs & JPEGs from the branch? Or committing them? Are the WebP's committed?
Based on what you've said, it sounds like you manually convert some PNGs to WebP, commit the WebP, but only the PNG is being processed. Is that correct? If so, that sounds like a bug. Could you please confirm?
Edit: Could you also please confirm that you are using the default configuration? (found on the README)
Hi, there is a commit action which happens after a user submits a PR, first converting PNG/JPG to WebP (deleting the original PNG/JPG in the process), then using calibreapp/image-actions to make sure the images are at least optimized to the desired quality, and then makes a new commit.
Here is the workflow, we aren't using default configuration right now, I can test with that if you would like: https://github.com/mastercomfig/hud-db/blob/f8ea05f3d40ea1d70fcb254829cbcd3dc65fdfe2/.github/workflows/autofix.yml#L19-L24
I am guessing it is an oversight with this logic: https://github.com/calibreapp/image-actions/blob/420075c115b26f8785e293c5bd5bef0911c506e5/src/get-changed-images.ts#L10
Which, as I understand it, receives GitHub API info about a PR if it exists in the current context, and gets the changed files from the PR. But, since we are changing those files again within the PR action, the API state is out of date with the current working tree of the action before the changes are pushed.
Thanks for the speedy reply @mastercoms 💨
What should happen in the (default) flow:
- You push on your branch
- Action runs, checks for changed image files on the head of the branch
- Action processes significantly compressed (>5%) image files, committing them to the same branch, leaving a comment on your PR.
Which, as I understand it, receives GitHub API info about a PR if it exists in the current context, and gets the changed files from the PR.
Your understanding is correct. If the action is run in the context of a pull request, then processes changed files within the head commit of your PR (checked in files, not untracked files). Failing to find any changed files, it then tries processing ALL files.
Some possible workarounds might be:
On your PR:
- Run your WebP conversion step, commit the WebP files to the branch (if updated/new)
- Run image-actions (separately), where it will see the new/changed WebP files, then process them
Run image-actions as a Docker step:
- Run your webp step as you already have it
- Run image-actions as a docker command, so that it won't have github/pr context
I haven't run image-actions as a raw Docker step for several years. It used to be possible, but I haven't thought about it for a while as this action is primarily designed for GitHub.
Admittedly, neither of these options are silky smooth. I just wanted to share them so there's some options to consider as an alternative.