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

Set up imgbot to automatically compress image assets #28157

Closed
roryabraham opened this issue Sep 25, 2023 · 20 comments
Closed

Set up imgbot to automatically compress image assets #28157

roryabraham opened this issue Sep 25, 2023 · 20 comments
Assignees
Labels
NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2

Comments

@roryabraham
Copy link
Contributor

HOLD on https://github.com/Expensify/Expensify/issues/299601

Slack context: https://expensify.slack.com/archives/C01GTK53T8Q/p1695410267279189

Problem

Manually compressing images before uploading is a tedious task. It is easy to forget about it. However, we're adding many uncompressed images to this repo for the HelpDot site.

Solution

Add a bot that will raise a PR to losslessly compress images - imgbot is free for opensource repos.

So even if we upload an unoptimized image, the bot will take care of it 🤖

A few stipulations:

  • It seems like we need to authorize imgbot as a GitHub Application. We currently have an Open Issue to create an open-source GitHub Application from OSBotify. So this is on HOLD for that
  • When we do turn it on, we want to preserve our team's time as much as possible. So let's create a workflow such that if OSBotify opens an imgbot PR, the GitHub Actions bot will automatically approve and merge it
@roryabraham roryabraham added Weekly KSv2 NewFeature Something to build that is a new item. labels Sep 25, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

@roryabraham
Copy link
Contributor Author

Technically this will apply to any image in this repo since imgbot also works for SVG images. So I'm adjusting the title accordingly.

@rushatgabhane let's please also stick with lossless, not aggressive/lossy compression.

@roryabraham roryabraham changed the title [HOLD] Set up imgbot to automatically compress HelpDot images [HOLD] Set up imgbot to automatically compress image assets Sep 25, 2023
@melvin-bot melvin-bot bot added the Overdue label Oct 4, 2023
@roryabraham
Copy link
Contributor Author

Still on HOLD. Working on getting OSBotify appified. Actively discussing in #deployer

@melvin-bot melvin-bot bot removed the Overdue label Oct 4, 2023
@melvin-bot melvin-bot bot added the Overdue label Oct 12, 2023
@roryabraham roryabraham changed the title [HOLD] Set up imgbot to automatically compress image assets Set up imgbot to automatically compress image assets Oct 17, 2023
@roryabraham
Copy link
Contributor Author

Taking this off HOLD

@roryabraham
Copy link
Contributor Author

@justinpersaud is going to help out with this 🎉

@justinpersaud
Copy link
Contributor

@roryabraham @muttmuure

Cloudflare has a product to automatically compress images and it is zero effort on our part.

https://developers.cloudflare.com/images/polish/

Cloudflare Polish is a one-click image optimization product that automatically optimizes images in your site. Polish strips metadata from images and reduces image size through lossy or lossless compression to accelerate the speed of image downloads.

When an image is fetched from your origin, our systems automatically optimize it in Cloudflare’s cache. Subsequent requests for the same image will get the smaller, faster, optimized version of the image, improving the speed of your website.

If the problem we're solving here is handling uncompressed images, we could just turn that on and see if it helps?

@justinpersaud
Copy link
Contributor

@roryabraham I installed imgbot in the App repo. Seems like it will open a PR with optimized images shortly. Waiting to see what that looks like.

@rushatgabhane
Copy link
Member

wohooo!

@rlinoz
Copy link
Contributor

rlinoz commented Oct 24, 2023

Hey, here is the PR #30230

Do we want to look for something specific in it?

@justinpersaud
Copy link
Contributor

thanks @rlinoz

I will take it for now. I think the plan is to automate merges so I'll grab it and figure out next steps.

@justinpersaud
Copy link
Contributor

@rushatgabhane do you want to take a look at it too? I don't see anything that stands out at an initial glance. I will tag Shawn on it too.

@rushatgabhane
Copy link
Member

is it okay to compress images inside ios and android folder? 📂

@rushatgabhane
Copy link
Member

rest looks good! ✅

@justinpersaud
Copy link
Contributor

Well lossless compression doesn't touch quality or dimensions, so I think it's fine. @AndrewGable maybe you have an idea if it's ok to compress images in the android/ios folders?

@justinpersaud
Copy link
Contributor

When we do turn it on, we want to preserve our team's time as much as possible. So let's create a workflow such that if OSBotify opens an imgbot PR, the GitHub Actions bot will automatically approve and merge it

Is this still true?

A few other tasks that need to be done here

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Oct 24, 2023
@justinpersaud
Copy link
Contributor

Submitted PR for the above tasks

#30270

@AndrewGable
Copy link
Contributor

It's OK to compress images in iOS/Android as long as the image dimensions stay the same.

@justinpersaud
Copy link
Contributor

This is mostly done, just doing some polish. The first imgbot PR was sent earlier and automatically merged:

#30383

@melvin-bot
Copy link

melvin-bot bot commented Oct 27, 2023

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@justinpersaud
Copy link
Contributor

I think we're all done here. We found a case where an optimized image had the colour changed slightly in the regression above. I ended up added the file to the ignore list for optimizations and reverting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2
Projects
Development

No branches or pull requests

6 participants