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

CI: Image optimization #2435

Open
Tracked by #2771
M123-dev opened this issue Jun 30, 2022 · 3 comments
Open
Tracked by #2771

CI: Image optimization #2435

M123-dev opened this issue Jun 30, 2022 · 3 comments
Labels
asset cache cached assets (SVG…) for the knowledge panels CI performance refactoring ⭐ top issue Top issue.

Comments

@M123-dev
Copy link
Member

M123-dev commented Jun 30, 2022

Problem

We got some svg optimization from @lsaudon in #2414.

It would be great if we could automate and expand it beyond svgs only.

My preference would be ImgBot which could also be interesting for other repos as well. Would you have a look at this @teolemon

Part of

@monsieurtanuki
Copy link
Contributor

My 2 cents...

I'm not a big fan of systematic optimization

  • they have side-effects (fix: Revert "refactor: Avoid unused parameters (#2407)" #2418, perf: Optimize SVG files #2414 (comment))
  • so far they are not tested (what about goldens test for SVGs?)
  • I didn't see any evidence of the gain in space (would be easy to measure) and performances (less easy)
  • in the case of SVG optim, I don't know what the source SVG files were (if they were those in assets/ they were already downgraded optimized files)
  • it distracts us from more important issues, especially now that the app is live

TL;DR
If it is tested, if it does optimize the app, if it's fast to code and easy to maintain, why not.

@AshAman999
Copy link
Member

so far they are not tested (what about goldens test for SVGs?)

Ran test on my fork for IMG bot and all test were passing ,doesn't seem to have any breaker for now to stop it from being implemented in our project
AshAman999#1

@monsieurtanuki
Copy link
Contributor

@AshAman999 I'm not sure the Smoothie tests include the correct display of the 233 SVG files (with the flutter limitations regarding SVG display). For PNG I assume we're safer and that flutter displays them correctly.

In addition to optimization, an added value would be to get the original files from the server and to put them in assets/. Cf. #1963.

@teolemon teolemon added CI asset cache cached assets (SVG…) for the knowledge panels labels Jun 30, 2022
@teolemon teolemon changed the title CI: Img optimization CI: Image optimization Jul 14, 2022
@github-actions github-actions bot added the ⭐ top issue Top issue. label Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
asset cache cached assets (SVG…) for the knowledge panels CI performance refactoring ⭐ top issue Top issue.
Projects
Status: 💬 To discuss and validate
Development

No branches or pull requests

4 participants