Revue de code : quel format choisir ?
Je venais de terminer le développement d’une fonctionnalité assez complexe qui m’avait occupé près d’une semaine. Cela avait impliqué un certain nombre de refactorings, faisant que la quantité de code modifié à revoir était plutôt conséquente.
Je déplace alors ma User Story dans la colonne Code Review du board, et prévient l’équipe que cette fonctionnalité est en attente de revue, au cas où quelqu’un puisse s'en charger tout de suite.
Ça tombe bien, Benoît est disponible et démarre immédiatement la revue. Il vient me voir un quart d’heure plus tard : “Hormis une petite erreur de nommage, tout est clair et fonctionnel”.
Trois semaines plus tard, un bug bloquant est découvert en production, sur un cas difficile à reproduire. Nous passons près de deux jours avant d'en déterminer l’origine.
Mais une fois que nous avons trouvé, nous nous sommes sentis un peu bêtes : l’erreur était évidente en lisant le code...
Il est possible que malgré les revues, de trop nombreux défauts continuent à être détectés trop tard, alors qu’ils semblent évidents une fois détectés.
Si la revue n’a pas été préparée par les relecteurs, il est probable que l’on passe trop rapidement sur le code pour que les défauts soient correctement relevés en séance. C’est ce qui s’est produit au cours de l’anecdote ci-dessus.
On précisait dans Best Kept Secrets of Code Review, SmartBear Software, 2006).
Il arrive également que l’auteur navigue lui même dans le code : c’est utile pour expliquer le code et faciliter la compréhension de l’intention derrière le code. Mais si on utilise ce mode de navigation pour toute la revue, il est possible que l’auteur navigue trop rapidement dans le code ou omette accidentellement d’en parcourir certaines parties, ce qui est d’autant plus risqué si la revue n’a pas été suffisamment préparée.
Nous avons rencontré un second écueil dans cette anecdote : le développement de la fonctionnalité a pris une semaine entière et beaucoup de code a été produit.
Revoir dix à vingt lignes de code peut se faire en quelques minutes, mais si la revue concerne beaucoup de code, il faudra logiquement y passer plus de temps.
Dans notre premier article, nous précisions qu'il est important de limiter la quantité de code revu en une séance, ainsi que la durée de ces séances. Au delà, les relecteurs ne détecteront que peu ou pas de défauts supplémentaires, car il est fort probable que l’attention ait diminué et qu’on laisse alors passer certains problèmes.
Afin de limiter la quantité de code à revoir, il est indispensable que ce soit une pratique régulière :
Les membres de l’équipe peuvent ne pas avoir le même niveau d’exigence, ou ne pas savoir quels défauts chercher en relisant le code.
Pour remédier à ce problème, on évoquait plus tôt la nécessité d’avoir des standards écrits et partagés.
Il est également très utile d’utiliser des checklists pour aider à mener la revue :
Un point de vigilance concernant l'outillage : si celui-ci peut nous aider en détectant automatiquement de nombreux défauts, il ne peut en aucun cas remplacer la revue de code. Même si un grand nombre de défauts potentiels peut être relevés automatiquement, il serait dangereux de penser qu'on peut se reposer intégralement dessus. Par exemple, certains points clés des principes Clean Code comme l'intention et le caractère explicite du code, exprimés par le nommage, nécessiteront toujours une relecture manuelle.
Enfin, on observe parfois que certains relecteurs n'osent pas relever des défauts, pensant qu’ils ne sont pas légitimes pour remettre en cause le code d’un autre, ou craignant une mauvaise réaction de l’auteur. Nous reviendrons plus en détail sur ce problème la semaine prochaine : il est important que le cadre de la revue soit bienveillant et propice aux échanges pour que cette situation ne se produise pas.
Si les défauts ne sont pas corrigés bien qu’ils aient été détectés, un élément fondamental a probablement été oublié. Une revue de code, quel que soit son format, devrait avoir :
Nous avons exploré quelques nouvelles pistes au travers de ce retour d’expérience :
Rendez-vous la semaine prochaine pour un nouvel épisode : nous parlerons des problèmes liés aux problématiques d'ego lors de la revue, du risque de la réserver aux experts, ou encore de manque de soutien du management.
Et comme précédemment, 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.