-
-
Notifications
You must be signed in to change notification settings - Fork 13
Optimize SVG icons #57
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #57 +/- ##
==========================================
- Coverage 79.46% 79.15% -0.32%
==========================================
Files 23 23
Lines 2518 2518
==========================================
- Hits 2001 1993 -8
- Misses 517 525 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
What is the point of this change? Small difference in file size shouldn't really matter as it's loaded from local disk, not over the network. And this PR removes also a bunch of metadata (including license) from some of the icons, and in few cases changes also default icon size. And the formatting change will be un-done the next time icon is edited anyway... If there is some reason to prefer smaller files anyway (or if I missed some other side effects), IMO better to not touch the source in the repo, but add the conversion to the build process. |
The above comment applies to other similar PRs too: QubesOS/qubes-desktop-linux-manager#265, QubesOS/qubes-manager#426, |
This PR is just small housekeeping, so it will not fix something or introduce new features. Cleaning redundant SVG codes will save some CPU, Mem, Disk usages, but indeed the difference is small. Feel free to close this if review burden is too much.
These are my mistakes and not intended. I will fix these problems ASAP if this type of PR can be accepted.
I think adding optimization step to build process is not needed, because icon files are not changed frequently, and adding external npm package to build env maybe risky. (There are many cases of npm package's compromise...) |
SVGO with --pretty --multipass option and removeMetadata: false cleanupIds: false removeViewBox: false
I changed SVGO config to keep metadata, viewbox and id. |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025072115-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025061004-4.3&flavor=update
Failed tests14 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/142375#dependencies 11 fixed
Unstable testsPerformance TestsPerformance degradation:8 performance degradations
Remaining performance tests:64 tests
|
using SVGO with --pretty option.