Généralement, on ne lit le code de ses collègues que quand il nous pète à la figure. Et, évidemment, c'est généralement déjà trop tard. Et généralement, ça arrive en plein rush de release. Mais généralement, on oublie rapidement comment c'est arrivé et on continue de bosser de la même manière. Généralement, on est sacrément cons.

La relecture de code (review de code, en franglish dans le reste de l'article) permet de palier à ce genre de problème. Le principe est simple : faire valider les modification du code source par quelqu'un d'autre. Généralement, une requête de relecture (review request) est envoyée lors d'un commit; cette requête est soit validée, soit rejetée - avec commentaires - par le relecteur.

Pour une équipe comme la notre (3 programmeurs), ça semblait un chouilla surdimensionné. Mais suite à un problème vraiment moche sur le projet, on a décidé de briser le cercle vicieux en mettant en place cette solution au studio.

La review de code aide, entre autres, à :

• garder un code de qualité,
• s'assurer du respect des conventions,
• partager efficacement les changements et nouveautés,
• permettre à toute l'équipe de s'approprier le code,
• responsabiliser l'équipe plutôt qu'un individu.

Alors oui mais comment ?

La plus grosse réticence est venue de la réputation fastidieuse de ce genre de process. Lourdeur, appréhension du changement, perte de dynamisme, flemme, cycles lunaires...

Il a donc fallu garder en tête que ça ne devait pas devenir une nouvelle contrainte gratuite. On a décidé : 1 commit = 1 relecture tierce. Mais la relecture se fait après coup : le commit n'est pas bloqué et intègre le dépôt tout à fait normalement. On a donc privilégié de garder la rapidité du commit, en prenant le risque de devoir revenir en arrière.

Reviewboard à la rescousse

On a choisi Reviewboard. Je ne vais pas vous faire sa pub, leur site s'en charge très bien. Le système est simple, c'est ce qu'il nous fallait. Avec un peu de bidouille en Python et quelques gouttes se sueur sur notre serveur, un post-commit-hook subversion génère automagiquement les requêtes lors d'un commit.

Pour gérer facilement les destinataires des requêtes, un groupe a été créé du même nom que l'utilisateur. Par exemple, une requête générée par mon commit tombe dans le groupe à mon nom. Les personnes censées vérifier mes commits sont abonnées à ce groupe. Ça permet d'avoir directement dans sa boîte de réception les requêtes qui nous sont destinées, et de mieux compartimenter qui relit le code de qui. C'est utile notamment lors de l'arrivée d'un nouveau membre dans l'équipe !

Listing des reviews générées par les commits

Exemple de review request

Une fois la requête générée, un des membres de l'équipe concerné la consulte et relit attentivement les changements. Si un problème est trouvé, il commente le code et soumet la review. Si tout est bon, il appuie sur le bouton "Ship it!".

J'ai ajouté la possibilité de mettre à jour une requête plutôt que d'en créer une nouvelle lors d'un commit, en cas de travail sur un seul sujet en plusieurs fois, ou de mini changements après coup. Ça permet d'avoir moins de requêtes dans tous les sens, et d'avoir plus de contexte sur une série de commits qui sont liés (donc une relecture plus efficace). La syntaxe est simple : il faut rajouter ">ur:" suivi du numéro de la requête à compléter quelque part à la fin du message de commit.

Choix du diff avec une review mise à jour plusieurs fois

Review, mise à jour de review et "Ship it!"

En pratique

Les premières relectures font mal. Il a fallu quelques sprints pour se faire au temps passé à bouquiner la prose de ses collègues, et ça prend quelques shots de tord boyaux pour bien digérer les retours des autres sur son code chéri "que non il est pas crade, qu'il est efficace d'abord, et que c'est toi qui n'y connais rien." Mais alors on ne va pas se le cacher, ça fonctionne vraiment bien. Ces chiffres le prouvent.

Preuve irréfutable de l'efficacité de la review de code

Les effets sont visibles assez rapidement en prenant du recul. L'ensemble du code se stabilise plus durablement, et l'équipe a une connaissance beaucoup plus pointue des recoins obscurs du code.

A la longue...

C'est bien Bisounours jusque là, mais il arrive inévitablement un moment où on s'enlise dans le gros rush, et petit à petit on laisse tomber la review de code. Au bout d'un certain temps, il a fallu à répétition combattre le retard pris, en ayant l'impression de ne pas s'en sortir.

Les causes sont multiples : manque de temps, temps de travail pas correctement matérialisé dans le planning, manque de courage pour relire des commits épais comme le Vidal, constipation, mauvais alignement des planètes, ...

Là encore, on n'a pas trouvé de solution miracle. On en a à répétition parlé pendant les rétrospectives, souvent pour en arriver à des "Faut juste arrêter d'être cons" pas très efficaces. Quelques actions qui ont amélioré la chose :

• Afficher un tableau de pointage reviewboard, pour arborer fièrement ses efforts pendant le stand up
• Faire des tâches de synchronisation en début de sprint : commencer par rattraper le retard de la semaine passée avant de commencer à commiter des tétrachiées de code
• Tirer les autres par le haut, quand on est motivés, faire de la pair-review

Ce constat remet aussi en question un des choix initiaux : le commit yolo (review après coup). C'est sûr que si les changements ne sont appliqués que si un collègue les a validé, ça change les priorités. Mais on perdrait une partie de notre réactivité, qui nous a été salvatrice si souvent...

En plus des effets connus et cités plus haut, on avait sous estimé la différence de qualité de travail quand on sait que quelqu'un va relire et valider (ou non) ce qu'on envoie. Ça fout la pression, mine de rien, et on fait mieux les choses.

Quelques limitations...

Reviewboard ne supporte pas les requêtes inter-dépôts. Ce qui signifie que le même travail sur le dépôt principal et en parallèle sur des externals ne peut pas être présenté correctement en une seule requête. C'est dommage, mais ce n'est pas la fin du monde.

Plus gênant : ce n'est pas aisé de voir ses requêtes qui ont été relues mais rejetées; pour une probablement bonne raison, ils ne sont pas mis en valeur dans la liste et le seul repère est le compte de reviews à droite et l'absence de drapeau vert "Ship it".

Bilan

Les problèmes rencontrés ne doivent pas occulter la terrifiante efficacité du process. Ça a tenu ses promesses et bien plus encore. C'est exactement le genre de décision où on se dit, à postériori: "Si c'était à refaire... je referais exactement pareil". Et, lecteur, probablement que toi aussi. (sauf constipation ou mauvais alignement des planètes ofc).