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.

Sign up for GitHub

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

Jump to bottom

Optimize images across the repo #7979

Closed
WSLUser opened this issue Oct 20, 2020 · 4 comments
Closed

Optimize images across the repo #7979

WSLUser opened this issue Oct 20, 2020 · 4 comments
Labels
Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@WSLUser
Copy link
Contributor

WSLUser commented Oct 20, 2020 •
edited
Loading

Description of the new feature/enhancement

Optimize images added from PRs automatically to reduce image size while retaining image quality. (Looking at #7971 in particular). This will help keep the disk size of Windows Terminal down when updating or adding new images.

Proposed technical implementation details (optional)

Add the yaml file. Also, if this is accepted, I have another image optimization PR I can do to optimize our current images (reducing the overall image size by 23% for this repo) which I'd prefer to do prior to adding this Action as the images were optimized from separate tooling and don't want any corruption added to the images (it does detect if an image doesn't require optimization).
@WSLUser WSLUser added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Oct 20, 2020
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 20, 2020
@DHowett
Copy link
Member

DHowett commented Oct 20, 2020

I'm interested in optimizing the existing ones, but I don't think it's necessary to add another action that enforces it.

For the terminal app resource directories, it looks like we save a grand total of ~42kb/flavor or about 11%. Where are you seeing the greatest savings?

@DHowett DHowett added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Oct 20, 2020
@WSLUser
Copy link
Contributor Author

WSLUser commented Oct 20, 2020

So the Action can't handle svg files so it skips those due to it's dependencies but using ImgBot, all png and svg files had a file reduction size. This is mostly to help reduce the footprint for hosts that don't have a bunch of storage available where even a few MB might matter. Compared to other things, it's probably not the biggest change that can help but it's a quick easy one. I did a visual inspection from several types of devices to verify the images looked the same.

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Oct 20, 2020
@WSLUser
Copy link
Contributor Author

WSLUser commented Oct 20, 2020

If you're not interested in adding the Action, I can repurpose this issue for general image optimization and link the PR against it.

@WSLUser WSLUser mentioned this issue Oct 20, 2020
Closed
6 tasks
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Oct 21, 2020
@DHowett DHowett changed the title Add Image Optimizer GH action Optimize images across the repo Oct 21, 2020
@ghost ghost added the In-PR This issue has a related PR label Oct 22, 2020
@WSLUser WSLUser mentioned this issue Nov 5, 2020
Merged
6 tasks
@DHowett DHowett closed this as completed in 3f75788 Nov 6, 2020
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Nov 6, 2020
@ghost
Copy link

ghost commented Nov 11, 2020

🎉This issue was addressed in #8169, which has now been successfully released as Windows Terminal Preview v1.5.3142.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@DHowett @WSLUser