potiuk edited a comment on issue #20740:
URL: https://github.com/apache/airflow/issues/20740#issuecomment-1010978696


   > Why do we have to run static checks with Breeze when it's possible to run 
via pre-commit. Only a few checks like flake8, mypy requires to build an image 
while others can be run. Is there any specific reason to support static-checks 
via Breeze commands?
   
   Actually the only reason is auto-complete. Even the few "image" pre-commits 
can be run with pre-commit (they will just fail in case image is not built). 
But I found this really problematic that someone who wants to run specific 
pre-commit has to look it up elsewhere (pre-commit has no support for 
auto-complete and apparently author of pre-commit is rather "cold" when it 
comes to supporting it. 
   
   The pre-commit's autocomplete issue is here 
https://github.com/pre-commit/pre-commit/issues/1119 (you can see my comments 
there) - 2.5 years and the response from @asotile was rather "cold" towards it 
(and he refused yaml parsing and click idea). While I love pre-commit in 
general, this is the only feature i actually miss. In our case the lack of 
autocomplete with our 100 (just counted !!!!) pre-commits where we continue to 
add them with a few pre-commits a month, the only way we can do it is by 
parsing the yaml. And since we are already using click and autocomplete in the 
new Breeze, I think this is the easiest way.
   
   > @potiuk Do you think we have to parse the .pre-commit.yml file on the fly 
and enable auto-complete for those checks rather than adding click options ?
   
   We can do something else. As @asotile mentioned in the comment - parsing the 
yaml on the flight might be slow. So one other option could be that we are 
using (yes!) pre-commit to generate list of pre-commits. We already do that 
actually in 
https://github.com/apache/airflow/blob/main/scripts/ci/pre_commit/pre_commit_check_pre_commit_hook_names.py
 where we check if all names of pre-commit are generated in 
`./breeze-copmplete` and documented in the docs. We could actually modify that 
one to generate a python file with list of pre-commits that we could import in 
Breeze (not only check it):
   
   Something like that: 
   ```
   PRE_COMMIT_LIST=[
      'identity',
      'check-hooks-apply', .
      ....
   ]
   ```
   
   then we could import it from that file and add as valid click options. That 
woudl likely be best way. 
   
   Additionally (not a must but should be easy), the whole index of pre-commits 
in our docs could be auto-generted. Currently we just check if the pre-commits 
are there, but instead (as we do with a few other auto-generated documentation) 
we could hard-code the names of tess that need breeze image and read the names 
and description from the yaml file and generate the table automatically: 
https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#available-pre-commit-checks
 
   
   UPDATE: 
https://github.com/apache/airflow/blob/main/scripts/ci/pre_commit/supported_versions.py
 - this is an example table of supported versions that we generate in 
pre-commit so w should do it in a very similar fashion.


-- 
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