-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: add workflow for automatic image compression on PR #126
Conversation
Images automagically compressed by Calibre's image-actions ✨ Compression reduced images by 47%, saving 5.03 MB.
239 images did not require optimisation. |
Images automagically compressed by Calibre's image-actions ✨ Compression reduced images by 1.6%, saving 864 bytes.
289 images did not require optimisation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool. Good job! 👏
Thanks @zebateira! Added @cwaring as a reviewer. |
Nice. Some of those saving are pretty big, as it is re-processing in a destructive mode we need to be a bit careful as it is introducing a lot of compression artefacts. A few thoughts:
|
@cwaring ouch, indeed some images are dreadful now.
It was fast and it didn't block the preview so I think we don't need to worry about that.
I don't think so, but I'll test this (it has a regex for only running it if the changes include images in the assets directory)
I think it would be more beneficial to have it on push so that we see the real final result in the preview.
Yeah, will change this 👍 |
Great, because we are lazy-loading all the images I'd prefer not to over compress. I ran ImageOptim over the original assets dir and that saved about 75% in a lossless way. |
Images automagically compressed by Calibre's image-actions ✨ Compression reduced images by 34.7%, saving 1.82 MB.
267 images did not require optimisation. |
I bumped the quality setting to 100% so we get lossless compression. Also reverted the previous compressions and the latest compressions do not seem to have lost any quality. Unfortunately this action does not allow to only compress the images that were changed in the PR. Issue here: calibreapp/image-actions#92 We do have the ImageOptim CLI, so in the future we can create an action that uses it and runs only on the changed images of the PR. If they don't implement it in the calibreapp/image-actions in the meanwhile. I suggest waiting and see if they implement it, and in the meantime if we get the change, we can simply create an action ourselves. |
If you end up doing this, do you think that you could report back to the issue in calibreapp/image-actions#92 ? Would be useful for the users over there who also want to use something that works in the meantime. |
@karlhorky will do! |
@cwaring, just checking in with you on this. Thanks! |
@zebateira would you mind dropping some notes here from Slack discussion. 🙏🏻 |
Yap!
This would be good to purse, considering the potential to spin this off into a separate github action that we could use across content heavy sites, but taking into account our bandwidth, I think we'll keep the implementation above, and then update in the future if necessary. |
Partially addresses #84
This PR adds integration with Image Actions to enable automatic image compression when a new PR is added.
So when the PR from staging is created with the new forestry changes, images will be automatically compress in the PR.