airflow-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [airflow] dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run automatically in pre-commit hooks
Date Sun, 11 Aug 2019 03:06:56 GMT
dimberman commented on a change in pull request #5777: [AIRFLOW-5161] Static checks are run
automatically in pre-commit hooks
URL: https://github.com/apache/airflow/pull/5777#discussion_r312721183
 
 

 ##########
 File path: CONTRIBUTING.md
 ##########
 @@ -583,67 +611,84 @@ class LoginForm(Form):
 # pylint: enable=too-few-public-methods
 ```
 
-# Git hooks
+# Pre-commit hooks
 
-Another great way of automating linting and testing is to use
- [Git Hooks](https://git-scm.com/book/uz/v2/Customizing-Git-Git-Hooks). For example you could
create a
-`pre-commit` file based on the Travis CI Pipeline so that before each commit a local pipeline
will be
-triggered and if this pipeline fails (returns an exit code other than `0`) the commit does
not come through.
-This "in theory" has the advantage that you can not commit any code that fails that again
reduces the
-errors in the Travis CI Pipelines.
+Pre-commit hooks are fantastic way of speeding up your local development cycle - you can
run exactly the same
+static code checks as are run in TravisCI but only limit the checks to the files you are
just committing.
 
-Since there are a lot of tests the script would last very long so you probably only should
test your
- new
-feature locally.
+You are *STRONGLY* encouraged to install pre-commit hooks as they speed up your development
and place less
+burden on the Travis CI infrastructure.
 
-The following example of a `pre-commit` file allows you..
-- to lint your code via flake8
-- to test your code via nosetests in a docker container based on python 2
-- to test your code via nosetests in a docker container based on python 3
+We have integrated the fantastic [pre-commit](https://pre-commit.com/) framework in our development
workflow.
+You need to have python 3.6 installed in your host in order to install and use it. It's best
to run your
+commits when you have your local virtualenv for Airflow activated (then pre-commit and other
+dependencies are automatically installed). You can also install pre-commit manually using
`pip install`.
 
-```
-#!/bin/sh
-
-GREEN='\033[0;32m'
-NO_COLOR='\033[0m'
-
-setup_python_env() {
-    local venv_path=${1}
-
-    echo -e "${GREEN}Activating python virtual environment ${venv_path}..${NO_COLOR}"
-    source ${venv_path}
-}
-run_linting() {
-    local project_dir=$(git rev-parse --show-toplevel)
-
-    echo -e "${GREEN}Running flake8 over directory ${project_dir}..${NO_COLOR}"
-    flake8 ${project_dir}
-}
-run_testing_in_docker() {
-    local feature_path=${1}
-    local airflow_py2_container=${2}
-    local airflow_py3_container=${3}
-
-    echo -e "${GREEN}Running tests in ${feature_path} in airflow python 2 docker container..${NO_COLOR}"
-    docker exec -i -w /airflow/ ${airflow_py2_container} nosetests -v ${feature_path}
-    echo -e "${GREEN}Running tests in ${feature_path} in airflow python 3 docker container..${NO_COLOR}"
-    docker exec -i -w /airflow/ ${airflow_py3_container} nosetests -v ${feature_path}
-}
-
-set -e
-# NOTE: Before running this make sure you have set the function arguments correctly.
-setup_python_env /Users/feluelle/venv/bin/activate
-run_linting
-run_testing_in_docker tests/contrib/hooks/test_imap_hook.py dazzling_chatterjee quirky_stallman
+You need Docker engine configured as the static checks are executed in docker environment.
You should
+build the images locally before installing pre-commit checks.
+
+## Installing pre-commit hooks
+
+`pre-commit install`
+
+Once you do it - all the commits by default will have to pass the static code analysis checks
in order to
+be able to add commit.
+
+
+## Docker images for pre-commit hooks
+
+Before you first install the pre-commit hooks you need to build docker images locally
+using `./scripts/ci/local_ci_pull_and_build.sh` to pull images from the repository
+or simply `./scripts/ci/local_ci_build.sh` to build the image based on an already pulled
image.
 
+Sometimes your image is outdated and needs to be rebuild because some dependencies have been
changed.
+In such case the docker build pre-commit will fail and inform you that you should perform
 
 Review comment:
   In this case the docker build pre-commit will fail and inform you that you should rebuild
the image.

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

Mime
View raw message