Comment rater vos revues de code ? – Épisode 2

Dans l’article précédent, nous avons commencé à voir pourquoi il est important d’être vigilant en organisant les premières sessions de revue.

Voici un nouvel épisode sur le thème de la revue de code, plus précisément sur les écueils que nous avons rencontrés et qui risquent de rendre difficile le bon déroulement de vos revues.

Cet article fait partie de la série :

Malgré la mise en place des revues, on croule toujours sous les bugs

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.

La revue n’a pas été préparée

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 l’article présentant les formats de revue de code que celle-ci doit être préparée. C’est important car la plupart des défauts sont relevés lors de cette relecture à tête reposée (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.

Le rythme est trop élevé

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 :

  • Si les User Stories ou les tâches ne sont pas assez finement découpées, on peut travailler avec le Product Owner pour trouver une granularité plus fine.
  • Si vous utilisez un processus de revue au fil de l’eau, n’hésitez pas à matérialiser une limite d’en cours sur la colonne correspondante : c’est une étape qui ne doit pas induire de blocage dans le flux. Certaines équipes utilisent cette limite pour déclencher une revue collective dès qu’il y a un stock de relecture suffisant.
  • Enfin et surtout, rien n’interdit de commencer la revue avant la fin du développement d’une fonctionnalité.

On ne sait pas quels défauts relever

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 :

  • Ces listes contiennent des éléments à vérifier systématiquement car identifiés comme critiques, ou parce qu’ils sont constatés régulièrement.
  • Constituez votre propre checklist et faites la évoluer au fil des revues. Voici un exemple d’une (vieille) checklist en Java ici.
  • De nombreux éléments de ces checklists peuvent être vérifiés automatiquement, via des outils comme SonarQubeFindbugs, Checkstyle.

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.

Les défauts détectés ne sont pas corrigés

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 :

  • un statut : le code est-il accepté en l’état ? Si non, quels sont les défauts à corriger ?
  • un suivi : les défauts relevés sont enregistrés (soit simplement en listant et leur correction après coup est vérifiée, si besoin au moyen d’une rapide seconde revue.

Alors, comment réussir vos revues ?

Nous avons exploré quelques nouvelles pistes au travers de ce retour d’expérience :

  • Veillez à bien respecter votre format :
    • préparer la revue
    • respecter un rythme soutenable
    • décider d’un statut en fin de revue et suivre la correction des défauts.
  • Utilisez des checklists pour guider la revue et ne pas oublier les points importants.
  • Automatisez ce qui peut l’être pour détecter des défauts.

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.