Skip to content

Conversation

@jbampton
Copy link
Member

"Lossless compression is a data compression technique where the original data can be perfectly reconstructed from the compressed data. In essence, no information is lost during compression and decompression, making it ideal for situations where data integrity is critical"

https://en.wikipedia.org/wiki/Lossless_compression

https://github.com/oxipng/oxipng

https://github.com/oxipng/oxipng?tab=readme-ov-file#git-integration-via-pre-commit

Ran pre-commit here locally and it compressed the images on first run.

So we have less data for some images with the same quality.

Less data means less to download etc and saves bandwidth.

Description

This PR adds another check / hook / test to our pre-commit framework.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

Ran locally pre-commit run --all-files

How did you try to break this feature and the system with this change?

"Lossless compression is a data compression technique where the original data can be perfectly
reconstructed from the compressed data. In essence, no information is lost during compression
and decompression, making it ideal for situations where data integrity is critical"

https://en.wikipedia.org/wiki/Lossless_compression

https://github.com/oxipng/oxipng

https://github.com/oxipng/oxipng?tab=readme-ov-file#git-integration-via-pre-commit

Ran pre-commit here locally and it compressed the images on first run.

So we have less data for some images with the same quality.

Less data means less to download etc and saves bandwidth.
@codecov
Copy link

codecov bot commented Jun 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 17.56%. Comparing base (b1851ba) to head (44a1ecd).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main   #11065   +/-   ##
=========================================
  Coverage     17.56%   17.56%           
  Complexity    15533    15533           
=========================================
  Files          5909     5909           
  Lines        529013   529013           
  Branches      64605    64605           
=========================================
  Hits          92912    92912           
- Misses       425652   425653    +1     
+ Partials      10449    10448    -1     
Flag Coverage Δ
uitests 3.58% <ø> (ø)
unittests 18.63% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DaanHoogland
Copy link
Contributor

looks good @jbampton , but how would enforcing this with github actions work? Will it refuse not optimised pictures or automatically compress them?

@github-actions
Copy link

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@github-actions
Copy link

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds oxipng, a lossless PNG compression optimizer, to the pre-commit configuration to automatically optimize PNG images during the commit process. The addition helps reduce file sizes while maintaining image quality, which can improve bandwidth usage and download times.

Key changes:

  • Adds oxipng hook to pre-commit configuration with optimization level 4 and safe stripping options
  • Enables automatic PNG compression as part of the development workflow

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm , please see my question @jbampton .

@jbampton
Copy link
Member Author

looks good @jbampton , but how would enforcing this with github actions work? Will it refuse not optimised pictures or automatically compress them?

From the official site: https://pre-commit.com/

"Git hook scripts are useful for identifying simple issues before submission to code review. We run our hooks on every commit to automatically point out issues in code such as missing semicolons, trailing whitespace, and debug statements. By pointing these issues out before code review, this allows a code reviewer to focus on the architecture of a change while not wasting time with trivial style nitpicks."

So pre-commit is designed to run on the developers local machine when running manually pre-commit run --all-files and also automatically on git commit. And these trivial issues like whitespace etc are found first on the devs machine before pushing up to GitHub for code review. Running it locally is best practice and speeds up development since you don't have to wait for the pre-commit GitHub action to fail. The devs on Sedona and Airflow all run pre-commit locally.

So we need to add some documentation somewhere about pre-commit, how to install it and work with it locally.

For this hook oxipng it just works on GitHub actions with a pass or fail. When running locally if the hook fails it does optimize the images and then you need to git add and git commit again. So we can still merge a PR if the oxipng hook fails if needed.

Some example docs we could add might be:

pre-commit

We run pre-commit with GitHub Actions so installation on
your local machine is currently optional.

The pre-commit configuration file
is in the repository root. Before you can run the hooks, you need to have pre-commit installed.

The hooks run when running git commit and also from the command line with pre-commit. Some of the hooks will auto
fix the code after the hooks fail whilst most will print error messages from the linters. If a hook fails the overall
commit will fail, and you will need to fix the issues or problems and git add and git commit again. On git commit
the hooks will run mostly only against modified files so if you want to test all hooks against all files and when you
are adding a new hook you should always run:

pre-commit run --all-files

Sometimes you might need to skip a hook to commit because the hook is stopping you from committing or your computer
might not have all the installation requirements for all the hooks. The SKIP variable is comma separated for two or
more hooks:

SKIP=codespell git commit -m "foo"

The same applies when running pre-commit:

SKIP=codespell pre-commit run --all-files

Occasionally you can have more serious problems when using pre-commit with git commit. You can use --no-verify to
commit and stop pre-commit from checking the hooks. For example:

git commit --no-verify -m "foo"

If you just want to run one hook for example just run the markdownlint hook:

pre-commit run markdownlint --all-files

@jbampton
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@jbampton a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

Copy link
Member

@vishesh92 vishesh92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm
@jbampton This is good. I believe that cloudstack website (https://github.com/apache/cloudstack-www) would really benefit from this same change since it has a lot of images.

@jbampton
Copy link
Member Author

jbampton commented Sep 16, 2025

clgtm @jbampton This is good. I believe that cloudstack website (https://github.com/apache/cloudstack-www) would really benefit from this same change since it has a lot of images.

I will add oxipng now to the website with pre-commit. Thanks

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15042

@jbampton
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@jbampton a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 15066

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15074

@harikrishna-patnala harikrishna-patnala added this to the 4.22.0 milestone Oct 24, 2025
@harikrishna-patnala harikrishna-patnala merged commit 024f89d into apache:main Oct 24, 2025
28 checks passed
@jbampton jbampton deleted the add-oxipng branch October 24, 2025 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants