Code style

Situation actuelle

• imports : un from .. import .. par module + tout les objets derrière,
• on surveille ce que dit "flake8" mais sans être contraignant,
• on se limite à 80 colonnes.

Discussions

Imports

Les lignes d'imports sont généralement sources de conflits lors des merges.
Pour réduire ça, on pourrait utiliser la convention de n'importer qu'un seul objet par ligne.
Ça peut faire des préambules un peu long, mais :

• si on rage trop, ça devrait nous encourager à "découper" les modules en plus petits modules,
• les IDEs et éditeurs sont généralement capables de "réduire" ces lignes d'imports, ou de commencer l'affichage après celles-ci.

On peut automatiser ça avec un plugin comme isort. Il peut en plus classer les imports par type, améliorant la lisibilité.

Note : Django utilise isort.

Linter

Chacun est libre de formater le code à sa sauce : flake8 ne vérifie que la PEP8, qui autorise de multiples formatages pour une même ligne de code.
Je vous propose d'utiliser à nouveau un plugin : black.

L'intérêt de ce plugin : rien n'est négociable et son formatage est déterministe (ou presque). Ainsi, Michelle sur sa machine peut obtenir le même formatage d'un module que Michel sur une autre machine.
Cela réduit énormément le nombre de conflits, et ça permet de moins se poser de questions qui n'ont pas un grand intérêt !

Pour cette raison, black est de plus en plus utilisé dans la communauté Python open-source.

L'intégration aux IDEs, éditeurs se passent généralement bien.
Seul point critique : il faut avoir python3.6 d'installer pour le faire tourner.

Nombre de colonnes

On voit l'intérêt des 80 colonnes : 2 fichiers ouverts sur un même (petit) écran, et c'est bien !
Mais c'est quand même très petit... Les auteurs de black ont l'air d'accord et propose, par défaut, d'utiliser une largeur de 88 colonnes.
Avec ça on peut encore ouvrir deux fichiers côte-à-côte sur un 14" ☺

Propositions

black

• Ajout de black à la CI en --check

• Au préalable, l'appliquer à toute la codebase, se fait "rapidement" :

  • un commit sur master avec l'ajout de la conf (fichiers à inclure, exclure),
  • passer black sur master,
  • cherry-pick le commit de conf sur chaque MR (et branche) et aussi appliquer black. → Et ça sans conflit supplémentaire !

isort

• Ajout de isort en à la CI --check
• Configurer isort en 1 import / ligne (on peut aussi garder le multi-imports par ligne, mais ça fait du conflit)

colonnes

• Passage au nombre de colonnes proposé par black (88), qui semble être
  un bon compromis pour du 14" en moyen/gros caractère avec 2 fichiers

à voir

On pourrait ajouter isort et black dans un hook de pre-commit, mais j'ai un peur que ça soit frustrant pour les nouveaux arrivants de pas réussir à commit pour ces raisons. (git commit -n ferait l'affaire)

• Du tooling se met en place pour gérer la conf et l'exécution de ce type d'outils.
  Si des solutions sont viables et très faciles à utiliser alors on pourrait les intégrer au projet pour avoir du hook au pre-commit, etc. Ça reste à étudier...