From 4969f31ce1240c56ce092a46cd883e5eba55beab Mon Sep 17 00:00:00 2001 From: agopi Date: Fri, 18 Jan 2019 12:28:34 -0500 Subject: [PATCH] Update ansible-lint execution Updated ansible-lint to run via pre-commit only on ansible files. Moved config file to its standard location, repository root, which simplifies syncronization and usage. Contains bumping ansible-lint to current version which also required adding few more rule excludes. These excludes are going to be removed one by one in follow-up changes. This gradual approach allow us to improve code style without endless merge conflicts. Config settings mostly based on those used by tripleo repos. Bumping linters can now be done by running 'pre-commit autoupdate'. Pro-commit always locks versions so there is no chance that a newer linter (ansible-lint) would break CI. Some documentation can be found at https://github.com/openstack/tripleo-quickstart/blob/master/doc/source/contributing.rst and applies mostly to any project using pre-commit. Co-Authored-By: Sorin Sbarnea Change-Id: I05eb561c4e353b5fe0bc7c6d3ab2f8ea6c6ea2f4 --- .ansible-lint | 15 +++++++++++ .pre-commit-config.yaml | 17 +++++++++++++ ansible/.ansible-lint | 15 ----------- doc/source/contributing.rst | 51 +++++++++++++++++++++++++++++++++++-- test-requirements.txt | 3 ++- tox.ini | 4 +-- 6 files changed, 84 insertions(+), 21 deletions(-) create mode 100644 .ansible-lint create mode 100644 .pre-commit-config.yaml delete mode 100644 ansible/.ansible-lint diff --git a/.ansible-lint b/.ansible-lint new file mode 100644 index 000000000..7b56ce587 --- /dev/null +++ b/.ansible-lint @@ -0,0 +1,15 @@ +--- +parseable: true +skip_list: + - '102' # [E102] No Jinja2 in when + - '204' # [E204] Lines should be no longer than 120 chars + - '206' # [E206] Variables should have spaces before and after: {{ var_name }} + - '405' + - '503' # [E503] Tasks that run when changed should likely be handlers + - '504' + - '601' # [E601] Don't compare to literal True/False + - '602' # [E602] Don't compare to empty string + - ANSIBLE0006 + - ANSIBLE0007 + - ANSIBLE0012 +use_default_rules: true diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 000000000..40ee491b7 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,17 @@ +--- +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v2.1.0 + hooks: + - id: mixed-line-ending + - id: check-byte-order-marker + - id: check-executables-have-shebangs + - id: check-merge-conflict + - id: debug-statements + - repo: https://github.com/ansible/ansible-lint + rev: v4.0.1 + hooks: + - id: ansible-lint + files: \.(yaml|yml)$ + exclude: ^rally/ + entry: ansible-lint --force-color -v diff --git a/ansible/.ansible-lint b/ansible/.ansible-lint deleted file mode 100644 index 90109fd3c..000000000 --- a/ansible/.ansible-lint +++ /dev/null @@ -1,15 +0,0 @@ -exclude_paths: - - ./gather/ -parseable: true -quiet: true -skip_list: - - skip_ansible_lint - - '204' - - '206' - - '405' - - '504' - - '601' - - '602' - - ANSIBLE0012,ANSIBLE0006,ANSIBLE0007,ANSIBLE0016,ANSIBLE0019 -use_default_rules: true -verbosity: 1 diff --git a/doc/source/contributing.rst b/doc/source/contributing.rst index 1888ec403..60e9ee0d4 100644 --- a/doc/source/contributing.rst +++ b/doc/source/contributing.rst @@ -18,6 +18,9 @@ You can view your public key using: $ cat ~/.ssh/id_*.pub +Setup +````` + Set your username and email for review.openstack.org: :: @@ -26,6 +29,7 @@ Set your username and email for review.openstack.org: $ git config --global user.name "example" $ git config --global --add gitreview.username "example" + Next, Clone the github repository: :: @@ -45,6 +49,10 @@ To set up your cloned repository to work with OpenStack Gerrit $ git review -s + +Making changes +`````````````` + It's useful to create a branch to do your work, name it something related to the change you'd like to introduce. @@ -54,8 +62,7 @@ related to the change you'd like to introduce. $ git branch my_special_enhancement $ git checkout !$ -Make your changes and then commit them using the instructions -below. +Now you can make your changes and then commit. :: @@ -66,6 +73,43 @@ Use a descriptive commit title followed by an empty space. You should type a small justification of what you are changing and why. +Local testing +````````````` + +Before submitting code to Gerrit you *should* do at least some minimal local +testing, like running ``tox -e linters``. This could be automated if you +activate `pre-commit `__ hooks:: + + pip install --user pre-commit + # to enable automatic run on commit: + pre-commit install --install-hooks + # to uninstall hooks + pre-commit uninstall + +Please note that the pre-commit feature is available only on repositories that +do have `.pre-commit-config.yaml `__ file. + +Running ``tox -e linters`` is recommended as it may include additional linting +commands than just pre-commit. So, if you run tox you don't need to run +pre-commit manually. + +Implementation of pre-commit is very fast and saves a lot of disk space +because internally it does cache any linter-version and reuses it between +repositories, as opposed to tox which uses environments unique to each +repository (usually more than one). Also by design pre-commit always pins +linters, making less like to break code because linter released new version. + +Another reason why pre-commit is very fast is because it runs only +on modified files. You can force it to run on the entire repository via +`pre-commit run -a` command. + +Upgrading linters is done via ``pre-commit autoupdate`` but this should be +done only as a separate change request. + + +Submit Changes +`````````````` + Now you're ready to submit your changes for review: :: @@ -82,6 +126,9 @@ use the amend feature after further modification and saving. $ git commit --amend $ git review +Changes to a review +``````````````````` + If you want to submit a new patchset from a different location (perhaps on a different machine or computer for example) you can clone the Browbeat repo again (if it doesn't already exist) and then diff --git a/test-requirements.txt b/test-requirements.txt index a6b35820a..6578a717c 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -4,11 +4,12 @@ hacking<0.11,>=0.10.0 -ansible-lint pykwalify coverage>=3.6 +pre-commit # MIT pytest==3.2.1 python-subunit>=0.0.18 + readme_renderer[md] sphinx>=1.3,!=1.6.1 oslosphinx>=2.5.0 # Apache-2.0 diff --git a/tox.ini b/tox.ini index 2815f3772..8efdc2554 100644 --- a/tox.ini +++ b/tox.ini @@ -18,9 +18,7 @@ whitelist_externals = bash extras = insights commands = {[testenv:pep8]commands} - bash -c "cd ansible; find . -type f -regex '.*.y[a]?ml' -print0 | xargs -t -n1 -0 \ - ansible-lint" \ - --exclude=rally + python -m pre_commit run -a pykwalify -d browbeat-config.yaml -s browbeat/schema/browbeat.yml pykwalify -d browbeat-complete.yaml -s browbeat/schema/browbeat.yml bash -c "set -e; for config in $(ls conf/); do \