Skip to content
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

Use Calibre's image actions to compress images in PRs #353

Open
zerolab opened this issue Nov 23, 2022 · 4 comments
Open

Use Calibre's image actions to compress images in PRs #353

zerolab opened this issue Nov 23, 2022 · 4 comments
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@zerolab
Copy link
Collaborator

zerolab commented Nov 23, 2022

https://github.com/calibreapp/image-actions

Previously done manually (e.g. #285)

@udbhavsomani
Copy link

Hi @zerolab, I am new to contributing to wagtail, shall I proceed with this issue if not resolved already?

@zerolab
Copy link
Collaborator Author

zerolab commented Jan 2, 2023

Hey @udbhavsomani thank you for expressing your interest. Go for it. The rule is that unless the issue or a comment on the issue explicitly says not to, and there is no-one else who is working on it, then feel free to leave a note that you are working on it, or just open a draft PR. See https://dev.to/lb/ten-tasty-ingredients-for-a-delicious-pull-request-hgc for a lot of useful tips

udbhavsomani added a commit to udbhavsomani/wagtail.org that referenced this issue Jan 2, 2023
@udbhavsomani
Copy link

Hi @zerolab, please review my PR: #359

@thibaudcolas
Copy link
Member

I’d like to propose we don’t do this via GitHub Actions, and instead do it manually. We can document the process to do it manually via something like Squoosh if we want, which should give the same image optimisation benefits without the drawbacks I foresee.

The drawbacks are:

  1. As-is, the Calibre action will always attempt to re-optimise images no matter how many times it’s already attempted in the past (see [Feature Request] Only optimized changed images calibreapp/image-actions#92). This is massively wasteful on CPU, which has a clear energy and environmental cost.
  2. If the Calibre action decides to optimise an image, then it’s an extra commit with the newly-compressed images. This effectively at least doubles the space images will take in the git history (as many times as there’s been optimisations). Whereas if we optimise more manually we can be more cautious about how many times we do so and whether the savings are worth the bloat in the git history.

The possible benefits also seem way too small to me:

  • There are 8 images currently in the repo that could benefit from this. None of them are particularly prone to change
  • For any new work on the site, we’d want to support dark mode, which almost entirely rules out usage of raster images for styling purposes (it’s much easier to theme SVGs).

So – if we want to improve on image size, my preference would be to:

  1. Convert all raster images we can to SVGs. For example we could push harder on SVG favicons.
  2. Document how we optimise the raster images we keep

@thibaudcolas thibaudcolas added help wanted Extra attention is needed question Further information is requested labels Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants