jbampton commented on PR #11065:
URL: https://github.com/apache/cloudstack/pull/11065#issuecomment-3113086217

   > 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](https://pre-commit.com/) with GitHub Actions so 
installation on
   your local machine is currently optional.
   
   The pre-commit [configuration 
file](https://github.com/apache/cloudstack/blob/master/.pre-commit-config.yaml)
   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`
   
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to