Arrêtons de perdre du temps sur les PRs

Arrêtons de perdre du temps sur les PRs

Par Olivier A. le 22 janvier 2019

 Lecture 5 minutes

Cet article fait suite au talk que j'ai donné lors du Meetup PHP #25 organisé par l'AFUP de Bordeaux pour clôturer la fin d'année 2018. Les sujets proposés étaient tous aussi intéressants les uns que les autres, nous avions comme sujets le déploiement via Deployer, une explication bien utile sur les Compiler Pass de Symfony, un guide de survie pour les lignes de commandes Linux ainsi qu'une réflexion sur "pourquoi et comment se passer de boucles dans nos scripts PHP ?" Enfin, le sujet qui concerne cet article, "Arrêtons de perdre du temps sur les PRs", est basé sur un retour d'expérience sur la mise en place d'outils d'analyse statique de code PHP chez Synolia.

Les retours faits sur les PRs ne sont pas toujours constructifs.

Même si nous étions tous convaincus de l'utilité des PRs (Pull Request), les retours faits par les reviewers (relecteurs) n'étaient pas toujours constructifs.

Quelques exemples de retours qui pourraient être évités dans les PRs "C’est quoi cet alignement des arguments ?"

  • "Le saut d’une seule signe suffit, inutile d’en faire deux" 
  • "Le corps de cette fonction est trop long, il faudrait la découper" 
  • "Cette variable ne décrit pas son contenu" (ex : $a)
  • "Cet attribut/fonction est défini(e) mais jamais utilisé(e)"
  • "Hummm, ça sent fort le copier/coller par ici !"

La réponse classique :

  • "J’ai pas le temps"

On entend aussi fréquemment dans l’open-space :

  • "Hey ! Qui a préfixé sa branche feaure !? Ça casse mon auto-completion !"

Tous ces retours - bien que légitimes - nous font perdre du temps ! En effet, le reviewer va devoir revenir sur la PR pour valider ces corrections et ces "détails" détournent son attention du code produit et des problématiques métiers. Puis, le commiter va devoir revenir sur sa PR pour corriger tout ça.

De superbes solutions existent

L'écosystème PHP est riche de nombreux outils d'analyse de code qui permettent de palier à ces retours (et même plus !) avant que le code ne soit relu. Voici quelques outils utilisés au quotidien par les équipes de Synolia :

  • PHPCS - CodeSniffer : Respect des conventions de codage (PSR-2, custom, …)
  • PHPMD - Mess Detector : Règles de Clean Code, Code Size, Naming
  • PHPStan - Static Analysis Tool : Analyse statique de code
  • PHPCPD - Copy/Paste Detector : Détection du copier/coller
  • PHP-CS-Fixer : PHPCS + fixe automatique des erreurs

Tous les outils que nous avons vu précédemment sont formidables, mais ils ne sont pas forcément utilisés, car lors de la mise en place de ces outils les équipes se heurtent souvent aux problématiques suivantes :

  • Des environnements de développement différents
  • Le manque de connaissance de ces outils par les développeur.se.s
  • Manque de rigueur des développeur.se.s ?
  • Lancer ces outils manuellement prend du temps

GrumPHP notre sauveur !

GrumPHP est un plugin composer qui permet d'exécuter un ensemble de tâches, et ce avec un minimum de configuration.

Installation : via composer avec "composer require --dev phpro/grumphp"

Configuration : un seul fichier, grumphp.yml à la racine de votre projet qui sert de configuration à tous ces outils.

Exécution :  un simple commit ! En effet, GrumPHP, lors de son installation, ajoute une ligne dans le hook de pre-commit de git, ce qui fait qu'il est exécuté avant chaque commit.

Toutes les taches prédéfinies : Ant, Atoum, Behat, Brunch, Clover, Coverage, Codeception, Composer, Composer, Require, Checker, Composer, Script, Doctrine, ORM, File, size, Deptrac, Gherkin, Git, blacklist, Git, branch, name, Git, commit, message, Grunt, Gulp, Infection, JsonLint, Kahlan, Make, NPM, script, Phan, Phing, Php7cc, PhpCpd, Phpcs, PHP-CS-Fixer, PHP-CS-Fixer, 2, PHPLint, PhpMd, PhpMnd, PhpParser, Phpspec, PHPStan, Phpunit, Phpunit, bridge, PhpVersion, Progpilot, Psalm, Robo, Security, Checker, Shell, XmlLint, YamlLint.
Mais vous pouvez également créer vos propres tâches ! C'est assez simple et en plus c'est du PHP ;)

En bonus :)

GrumPHP, dispose aussi de tâches "internes" que nous utilisons chez Synolia car elles sont très pratiques.

Git Blacklist : Détecte l’utilisation de “mots” blacklistés.
Git branch name : valide le nom de la branche via une expression régulière.
Git commit message : valide le message de commit via une expression régulière.

Les limites

Vous l'aurez peut-être remarqué, je n'ai pas parlé de PHPUnit au cours de cet article, et pourtant nous l'utilisons bien chez Synolia. Néanmoins, nous avons fait le choix qu'il devait être lancé manuellement par les développeur.se.s, et il est également lancé par notre CI (Intégration Continue via Jenkins). Ce choix a été motivé par le fait que nos tests PHPUnit ne font pas uniquement des tests unitaires (nous faisons également des tests fonctionnels et d'intégrations) et cela est trop long pour être lancé à chaque commit ;).

Même si GrumPHP marche très bien avec nos environnements de développement et nos habitudes (commit en ligne de commande ou via l'IDE) le hook créé automatiquement lors de l'installation de celui-ci fait qu'il est lancé sur la machine du/de la développeur.se, il nous faut donc la modifier manuellement (pour l'instant ?) pour qu'il soit lancé dans nos containers Docker.

Conclusion

L'équipe Synolia a généralisé l'utilisation de GrumPHP sur ses projets Symfony et E-Commerce avec des configurations différentes par typologies de projets. Grace à cela nous avons réduit les retours sur les PRs et donc réduit le temps de celles-ci, ainsi qu'augmenté encore un peu plus la qualité de code produit par nos équipes.

Pour aller plus loin :

GIF