-
Notifications
You must be signed in to change notification settings - Fork 342
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
Automatically run oxipng
on example screenshots
#1547
Automatically run oxipng
on example screenshots
#1547
Conversation
By comparing the artifact sizes of the last official GA workflow run and the one run on this PR, we can see noticeable reductions:
It's worth noting that the sizes from the current workflow aren't the same as image sizes that would be used on the website, given the fact that @mockersf used
A few examples: |
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.
LGTM.
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.
LGTM, just left a note for optimizing speed
With @janhohenheim's suggestion applied, |
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.
Could you remove cargo-install
usage?
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.
Looks good, thanks for doing this!
I have two notes for how to improve the workflow, but they're nits that don't prevent this from being merged. :)
@alice-i-cecile would you mind merging this, when you have the chance? I think it was missed in today's merge train. (Which is understandable, given how many PRs you had to merge today!) |
Update Screenshots
GitHub Action.oxipng
and running it with the maximum compression level) adds about a minute to the workflow runtime, which is minuscule when compared to the avg. ~3h completion time.