Contributors mailing list archives
Re: Linting for 13.0 branches (using black and pre-commit)by
Camptocamp SA, Simone Orsi
we are already using it and we love the tool, so: +1
Just an important thing: we should pay attention to documentation and ease of bootstrap this for newcomers.
While I love the tool, I fear that it could be a barrier for ppl not used to it.
For instance, how are you planning to make ppl install dependencies in a unique and reliable way? Via each repo/requirements.txt?
Thanks for your work and your insights :)
On Mon, Oct 7, 2019 at 12:56 PM Stéphane Bidoul <email@example.com> wrote:
Dear contributors,Following a recent conversation on twitter, and further talks during #OCAdays, there seems to be a wide interest for using Black  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  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 futureThe 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)
- 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/filesIn 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
Acsone SA/NV, Stéphane Bidoul