-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
chore: optimize PNG images to reduce repo size #15748
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
Conversation
- Compressed 88 PNG images using pngquant + optipng - Reduced images from ~70MB to ~18MB (~52MB savings) - All PNGs previously over 1MB are now under 1MB - This helps address the Vercel 250MB serverless function limit Note: Large GIF files (29MB total) still need attention - recommend converting to MP4/WebM for additional savings
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bundle ReportChanges will increase total bundle size by 465 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-server-cjsAssets Changed:
view changes for bundle: sentry-docs-client-array-pushAssets Changed:
|
|
This is pretty great but I think we need a permanent solution where we add the optimization step to the build. This way we can keep the originals and don't rely on people remembering (they won't) to optimize the images. |
jaffrepaul
left a comment
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 a nice bump to combat the issue for now. Images are the sole offender here, I started a branch to fix this that I didn't get merged. I'll dig that up and see if I can't make moves there.
Ultimately a lot of these old UI images need to be updated any way. Agree there should be something added to the build step to help govern image size.
Let's pull out the codeowner bit to it's own PR to keep this one single responsibility.
| # Remove @ symbol from reviewer name | ||
| REVIEWER_NAME=${REVIEWER#@} | ||
| if [[ "$REQUESTED_REVIEWERS" == *"$REVIEWER_NAME"* ]]; then | ||
| echo " - $REVIEWER_NAME is already a requested reviewer, skipping" |
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.
I would pull this out to its own PR since it's not related and would be easier to isolate if needed
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.
Oh yeah, sorry, I was playing around with it and forgot to remove it.
376cfc5 to
1e9e5a6
Compare
Note: Large GIF files still need attention. Will open a separate branch to address.
DESCRIBE YOUR PR
Tell us what you're changing and why. If your PR resolves an issue, please link it so it closes automatically.
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes: