Comment rater vos revues de code ? - Épisode 1
Dans le précédent article, nous avons présenté la pratique de la revue de code ainsi que deux formats que nous utilisons sur nos projets.
Mais introduire une nouvelle pratique avec succès n’est pas une chose aisée. C’est un peu comme mettre une barque à la mer : une fois dans l’eau, les premiers mètres sont assez chaotiques. Il y a beaucoup de vagues, on commence à se demander si c’était une bonne idée. Ne serait-il pas plus sage de retourner au rivage ? Mais en persévérant, on arrive finalement au large, où la mer est plus calme : il suffisait de s’accrocher.
Nous avons rencontré dans nos expériences de revue de code plusieurs écueils qui nuisent aux bénéfices attendus et qui peuvent mettre en péril la pratique dans l’équipe.
Voyons au travers de quelques anecdotes vécues* pourquoi vos premières revues de code risquent d’être difficiles et quels sont les fondamentaux nécessaires à des revues de code réussies.
Cet article fait partie de la série :
- Revue de code : quel format choisir ?
- Comment rater vos revues de code ?
* Toute ressemblance avec des personnes existantes ou ayant existé n’est peut être pas fortuite. Si vous vous y reconnaissez, nous pouvons peut être vous aider.
Mon équipe ne progresse pas
Au sein d’une équipe que je venais de rejoindre, j’entends un jour Bob râler devant son poste :
“Mais il n’est pas possible Martin, combien de fois a-t-on dit qu’on ne construisait plus les requêtes SQL avec des Strings mais avec des Criteria Hibernate !”
Je suis allé voir Bob pour comprendre ce qu’il se passait. Il m’explique alors qu’il a revu du code écrit par Martin et qu’il a encore dû corriger une requête SQL qui ne répondait pas aux standards.
Moi : “Mais, ils sont écrits quelque part vos standards ?”
Bob : “Non… mais on les connaît, Martin est le seul à ne pas les respecter. Bon en même temps il est arrivé récemment.”
Moi : “Effectivement… Mais j’ai une autre question : tu as dit que tu as dû corriger le code toi même ?”
Bob : “Oui, on fait comme ça, je préfère corriger moi même pour être sûr. Et je ne vais pas aller l’interrompre pour ça.”
Moi : “Et si on allait lui en parler ?”
La revue n’occasionne pas d’échange
Dans cette conversation, pourquoi Martin n’a-t-il pas intégré le standard ?
Relire le code de quelqu’un, c’est l’occasion de lui donner un feedback sur son code, et de l’aider à améliorer ses pratiques. Sans échange avec l’auteur du code, le relecteur a beau détecter les défauts, ceux-ci réapparaîtront inlassablement dans le code futur car l’auteur n’a pas obtenu de retour dessus.
La revue de code doit être préparée par le relecteur, au moyen d'une première relecture pour relever des défauts ou des questions à poser. Ceci permet d’en effectuer une partie en asynchrone, mais le risque est qu’elle se termine sans occasionner cet indispensable échange, idéalement verbal.
L’auteur ne corrige pas les défauts
L’exemple précédent montre aussi un manque de confiance de la part du relecteur envers l’auteur du code. C’est dommage car le meilleur moyen de progresser, d’apprendre et intégrer un standard est de l’appliquer soi même.
Si la revue de code est l’occasion de transmettre du savoir, c’est aussi le cas de l’étape de correction des défauts :
- il est donc important que ce soit l'auteur qui corrige les défauts relevés.
- plutôt que le relecteur corrige les défauts par manque de confiance, on peut démarrer une séance de Pair Programming et construire la correction à deux.
Il s’agit aussi de responsabiliser l’auteur du code : corriger les défauts d'un autre développeur peut l'infantiliser, alors que l'aider à les corriger lui même le responsabilise.
Des standards non partagés
L’utilisation de standards de code est une autre pratique essentielle pour parvenir à des revues efficaces. Ces standards sont construits collectivement par l’équipe, et écrits par exemple dans un wiki.
S’ils ne sont pas écrits, on va forcément les oublier ou les réinterpréter, et donc ne pas les assimiler. Ces standards, on les initie dès le début d’un projet, et on les fait évoluer dès que nécessaire, par exemple lors d’une “heure tech” récurrente.
Mes revues se transforment en débats sans fin et autres “trolls”
Pendant la première revue de code collective organisée au sein de l’équipe :
Kent : dis voir Becky, à la ligne 984, ton accolade n’est pas à la ligne, et en plus ta variable $moneyList ne respecte pas la casse standard : ici on utilise du snake_case !
Becky : alors, d’une part il n’y a pas de “casse standard”, je ne suis pas d’accord. Et de toute façon c’est has been ! Si on suivait la norme, on serait en PSR 42 et il n’y aurait pas de question à se poser !
Kent : Mouais il y a des choses bien dedans, mais je ne suis pas d’accord avec leurs règles de nommage !
La discussion dure encore un bon moment, arrêtons-nous ici.
Qu’elles soient au format collectif ou en binôme, les premières revues de code organisées au sein d’une équipe sont souvent “polluées” des débats interminables, généralement sur des points censés être simples.
Afin de couper court à ces débats, il est indispensable d’établir au plus tôt des standards écrits, comme on l’a vu auparavant. Et ceci surtout pour des choses pouvant paraître anodines comme le nommage (quelle norme PSR utilise-t-on en PHP ?) ou des particularités controversées du langage (faut-il utiliser var en C# ?).
Il est peu probable que toute l’équipe soit parfaitement alignée sur ces points. Chez OCTO non plus nous ne sommes pas d’accord sur tout. L’essentiel est de trouver un compromis, et de l’écrire dans les standards.
Malgré cela, on rencontre aussi des aspects non traités par les standards, ou des points plus compliqués qui nécessitent une discussion avant de décider d’une correction.
C’est pour cela que, pour conserver une revue efficace :
- Quel que soit le format, toute discussion ne concernant pas la détection d'un défaut est reportée après la revue.
- En revue collective, on ne passe pas plus d'une minute de discussion par défaut trouvé. Un modérateur et un gardien du temps y veillent.
Si un point fait débat, on le note dans une liste "à décider, non standard" : le tout est que le débat n'ait pas lieu au cours de la revue.
Alors, comment réussir vos revues ?
Au travers de ces deux retours d'expériences, nous vous avons déjà donné quelques pistes, mais s’il n’y en avait qu’une à retenir la voici : soyez vigilants en organisant vos premières sessions de revue.
Si elles se passent mal dès le début et que vous ne résolvez pas rapidement les problèmes, vous vous tirez une balle dans le pied. La pratique risque alors d’être abandonnée ou déformée.
Voici les points clés que nous vous proposons :
- Si votre équipe est novice avec la revue de code, commencez par vous former et privilégiez des sessions collectives, au moins dans un premier temps.
- Formalisez votre processus de revue de code et veillez à bien le respecter : comment doit-elle se dérouler ? quand ? avec quels supports ?
- Faites évoluer votre pratique de la revue au cours de vos rétrospectives.
- Appuyez-vous sur des standards que vous faites évoluer et constituez des checklists.
La semaine prochaine, nous vous proposerons un nouvel épisode riche en rebondissements : Malgré la mise en place des revues, on croule toujours sous les bugs.
En attendant, n’hésitez pas à partager vos retours d’expérience via les commentaires, qu’ils se soient bien ou mal passés !
Merci aux OCTOs de la tribu Software Craftsmanship pour leurs précieuses contributions.