[GitHub] [airflow] potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#issuecomment-521238624 Thanks @Fokko -> There are more checks to come, the whole idea is that we can add many more static checks (I have the follow-up PRs adding them) so it will be difficult to maintain such list. It's better to have one job covering all the checks (then the overhead for starting the job will be much smaller). We will run all the checks here except docs build (this is not really applicable for incremental pre-commit checks) and pylint (not applicable until we finish pylint introduction and get rid of pylint_todo.txt). So I prefer to leave that name. @dimberman - are you happy with my fixes to docs ? I would love to merge that one so that I can better handle the follow-up changes with more static checks and get them added one-by-one 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#issuecomment-520385868 And now it's also locally a bit simpler when your check requires rebuilding of the image (for example when you changed setup.py). It will fail and print the message informing that you should add REBUILD=true in front of your command to make it works or that you should run the git message with --no-verify switch: ``` ERROR! Quitting. The images need rebuilding. You should re-run your command with REBUILD=true environment variable set For example 'REBUILD=true git commit' or 'REBUILD=true git push' In case you do not want to rebuild, You can always commit the code with --no-verify switch. This skips pre-commit checks. Still, the tests will be run on CI will run the checks for you. ``` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#issuecomment-520384668 Hey @dimberman (and others ;) - @BasPH ). I made a slight improvement to pre-commit framework. Now it is not only used by pre-commit hook but also for travis CI tests (with --all-files switch) for all static checks in one job (except pylint). This saves quite a lot of overhead for booting VMs on travis - we have less jobs to handle and it's rather fast. Also as we add more static checks they will be run automatically as we add them in the same job. And people will get used to the pre-commit interface showing passsed/failed. See https://travis-ci.org/apache/airflow/jobs/570789001 for the job that runs the static checks. I have more separate PRs to add more static checks which I will add separately. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#issuecomment-520221392 Thanks @dimberman! Very good comments. I improved the docs quite a bit now. Note also that I am not too worried about having perfect documentation just yet :). I know I am not the best both from grammar and writing instructions to developers (I make too many assumptions about the things I know), but this is going to change hopefully. We have been granted two Technical Writers by the Google Summer of Docs initiative and one of the writers (Elena) decided to work on my proposal (I am a mentor in the program) to improve the whole on-boarding documentation for new users. That including Breeze (sooon!) and development environment in general. So over the next few months, I will be working with her to improve that part significantly - both from the English/grammar point of view and the way how the whole information is structured. I hope to learn a lot from her !! (she is a senior technical writer from Intel). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#issuecomment-520168998 All documentation and instructions are in https://github.com/PolideaInternal/airflow/blob/pre-commit-hooks/CONTRIBUTING.md#pre-commit-hooks 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#issuecomment-520167962 @nuclearpinguin ^^ also you can help to see if that works as expected :) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
potiuk commented on issue #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks URL: https://github.com/apache/airflow/pull/5777#issuecomment-520167375 Hey. While flying to NYC I managed to rebase, cleanup and trim-down pre-commit hook implementation for all our static checks. I would love if you can take a look, review and try it out. It is built on top of the CI image (and the scripts that build it locally). It is super easy to install and you can use it to run your checks locally. The best thing is that it runs automatically on multiple processors and only for the files you change in your commit - so it is really fast to run pylint + mypy + flake + docker lint (and you do not have to remember which files you should run it on. And it is super easy to trigger it manually. I think if we promote it within community that might significantly improve cycle time for development (especially taking into account TravisCI slowness). The checks are using exactly the same docker image and the same configurations that Travis CI uses - which gives more confidence (not 100% but close) before you push to Travis. I am happy to iterate on it a bit while I am starting holidays as this could be really useful to get better development experience. Once we merge this one I have a number of other pre-commit hooks that we can add over time (validating xml files, yaml, shellcheck. tab removal etc.) - but those can be added gradually - for now I focused on having our current checks in pre-commit. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
