1. Mailing Lists
  2. Contributors
  3. Linting for 13.0 branches (using black and pre-commit)

Contributors mailing list archives

contributors@odoo-community.org

Browse archives

2019 OCA Extra Delegates Campaign is NOW OPEN

[OXP] Odoo Merge Sprint

Avatar

Linting for 13.0 branches (using black and pre-commit)

by
Acsone SA/NV, Stéphane Bidoul
- 07/10/2019 12:51:15
Dear contributors,

Following a recent conversation on twitter, and further talks during #OCAdays, there seems to be a wide interest for using Black [1] as a code formatter for OCA code.

This post explains the proposed approach to implement it in OCA, as well as several improvements to code linting tools.

In a nutshell, Black is a python code formatter that quickly gained traction in the Python community in the last couple of years. It formats the code for you with a predefined set of rules and almost no options so as to free your mind for more important matters.

At #OCAdays, we discussed on how to put it in practice, and it was proposed to take the opportunity of this change to introduce pre-commit [2] as the linting framework for OCA in the 13.0 branches.

TL;DR: If you are not interested in the nitty gritty details, the only change you will need to remember compared to your current workflow is:
  • run Black before committing your code on the 13.0 branches
  • sort imports (stdlib, odoo, odoo.addons, local imports)
  • To save time, install pre-commit and run pre-commit install after cloning an OCA repository on the 13.0 branch. Linting, running black and sorting imports will be done automatically for you when you commit. You can also run it manually with 'pre-commit run --all-files'.
If you are interested in practical details and the rationale, read on.

pre-commit is a tool that lets contributors easily run lint checks locally in a consistent and reproducible manner. These checks are then run again by Travis to be sure conventions are enforced. In principle this is similar to what we are doing now with MQT LINT_CHECK="1", with the following beneficial differences:
  • installation of the linters locally is easier: only one tool needs to be installed on developer's machines: pre-commit, which will then take care of installing all the required linters for you
  • you then run "pre-commit install" after cloning a repository
  • lint checks are then run automatically when creating a commit locally so you can detect errors much earlier, without having to wait for Travis to detect them for you
  • some linters (such as Black, and the import sorter) fix the code for you, so you immediately have the diff available with all fixes to commit
  • all configurations are stored locally in the repository using default configuration files (.pylintrc, .flake8, etc); this means your favorite IDE will pick them up automatically to give you assistance in detecting and fixing errors
  • a wide variety of pre-commit plugins exist, which means we can easily benefit from them and progressively add more in the future
The proposed configuration includes the following linters:
  • black, with default options (to facilitate usage)
  • auto fix trailing whitespace
  • auto fix newline at end of files
  • detect debug statements
  • flake8 with same options as before (except line length which is now the 88 black default)
  • pyupgrade, to encourage python 3 idioms
  • pylint with mandatory checks (with some more mandatory checks than before, for v13 only)
  • pylint with all checks (optional + mandatory), not failing
  • isort to automatically sort imports (except in __init__.py where import order might be significant)
  • run eslint as a pre-commit hook, with the same configuration as pylint-odoo except that all checks are warnings. This is the same as what pylint-odoo does (javascript checks are not blocking).
You can see an example configuration in the storage project [3], the queue project [4] as well as partner-contact [5]. Look for the following files: .pre-commit-config.yaml, .flake8, .pylintrc, .eslintrc. Also look at the updated .travis.yml to see how pre-commit is run. To try it out,
  • checkout these branches
  • run 'pre-commit install'
  • start committing lint failures to see the effect.
If you have suggestions to make on the linter configurations, you can make them on this MQT PR: https://github.com/OCA/maintainer-quality-tools/pull/613/files

In practice, if there are no objections, the plan is to setup this configuration on all 13.0 branches this week, while there are not too many v13 modules merged. To do this, I will push all linter configuration files to 13.0 branches, as well as resetting the travis configuration to the default.

When the configurations will need to evolve later, the maintenance of these configuration files will become a bot task. As before, there will be no possibility to configure linting rules per repo.

Looking forward to reading your feedback.

Best regards,

-sbi


Follow-Ups