You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/01/12 12:07:21 UTC

[GitHub] [airflow] potiuk commented on issue #20740: Breeze: Running static checks with Breeze

potiuk commented 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 actually 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 both actually. 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 namse 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. We could hard-code the names of tess that need breeze image and read the names and description from the yaml file: https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#available-pre-commit-checks 
   
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org