[GitHub] [airflow] feluelle commented on a change in pull request #5070: [AIRFLOW-XXX] Add a git pre-commit hook template

[GitBox <gi...@apache.org>]

feluelle commented on a change in pull request #5070: [AIRFLOW-XXX] Add a git pre-commit hook template
URL: https://github.com/apache/airflow/pull/5070#discussion_r273708481
 
 

 ##########
 File path: .githooks/pre-commit
 ##########
 @@ -0,0 +1,74 @@
+#!/bin/sh
+
+# This Git Hook is based on the .travis.yml to avoid as many errors in the Travis CI Pipeline as possible.
+# It is optional, but recommended to have at least the pre-tests section enabled.
+
+# Please make sure that you have your development environment setup and ready. See the CONTRIBUTING.md for more information.
+
+# NOTE: You need to set the following variables before running this hook.
+# A python virtual environment is needed for the pre-tests to run in.
+VIRTUAL_ENV_PATH=/Users/feluelle/venv/apache-airflow/bin/activate
 
 Review comment:
   > However to me it feels weird having to edit the pre-commit hook for each feature to set the specific test, and requiring running Docker containers with a specific name. At the very least, I'd add some documentation on usage.
   
   You can of course just uncomment the `run_tests` section if you dont want it to run before each commit. I personally like it. The whole pipeline lasts like 3 min. I can understand you if you say you want to have a commit quickly done. But for me its the feeling that I am almost certain that the code I commit passes Travis and I dont need to check it every time.
   I have two docker containers one for python 3 and one for python 2 that I use to run tests, but if you like I can add some lines to run the tests in the python venv as well or instead.
   
   > In my experience, pre-commit hooks with long running processes is something that annoys people, rather than helping, so I don't think many people would use it in the this way. But feel free to clarify if your experience is different :)
   
   When I implement a new feature at first I edit the pre-commit hook change it once to the name of the new test class I am going to create along with the feature class. I am doing a kinda test-driven approach splitting the screen into one half containing the test class and one containing the feature. I can focus completly on writing logic for the feature and its tests instead of having to think about whether it works for python 2 or 3 or having some kinda linting issues I forget to check, etc. - I am done when the commit comes through. I can checkout to a commit that has a working progress of a feature. It does not need to be finished but it works until this point.
   
   
   
   **tl;dr**
   I think you can use this script even if you don't use it as git pre-commit. In PyCharm I have a Run Config running this script - I only need to click a button. But in case I forget to click it right before I commit changes there might be an issue in Travis that could probably have been avoided when I had used it as git pre-commit hook.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services