Comment bien donner et recevoir une code review (sans drama)

La code review est l’un des outils les plus bénéfiques et formateurs pour un développeur. Sauf quand c’est mal fait. Là, ça devient vite le drame. Il y a des choses à savoir humainement et techniquement pour réussir une code review. Que ça soit pour la donner ou la recevoir.

Litige

Il y a bien longtemps, dans une galaxie lointaine, très lointaine je bossais dans une équipe d’une dizaine de développeurs. Lorsque du nouveau code devait être intégré dans le système, il passait d’abord par la grande instance de la code review.

La juridiction compétente pour nous autres développeurs.

Et ce jour-là, c’est exactement ce qui se passe. Un de mes collègues envoie sa code review à un autre. Les discussions restent entre ces deux personnes. En apparence, tout se passait comme les textes de loi l’avaient prévu. En vérité, cette review n’était pas comme les autres.

Dans la confidentialité de cette pull request, le plus grand drame de l’équipe venait de commencer.

Au cours des jours suivants, j’ai senti que l’ambiance se détériorait. Progressivement, les visages se sont fermés. La communication a pris du sursis.

Un beau matin, le collègue qui avait reçu la code review se tourne alors vers moi.

« – Je t’ai assigné la code review de Mr. X. J’en peux plus, ça me soule. Y’a rien qui va et il fait pas ce qu’on lui demande. Ça fait plusieurs jours, je veux plus avoir à gérer ce bordel. »

J’ai pas vraiment eu de réaction. Je ne comprenais pas encore ce qui se passait. En ouvrant cette fameuse PR sur GitLab, j’ai vite compris.

Que ça soit d’un côté ou d’un autre, c’était exactement ce que tu ne veut pas faire ou dire lors d’une core review.

Énormément de code. Aucun contexte. Pas de tests. Des commentaires violents. Des débats sans fin. Ça durait depuis une semaine et ça grossissait à vue d’œil. Y’avait le feu partout et les dégâts étaient déjà graves.

Cette code review à elle toute seule sera le point de départ du drame qui transformera l’équipe.

Cour d’assises

Si j’utilise autant le champ lexical de la justice dans l’audience que tu m’accordes aujourd’hui, c’est pour une bonne raison. Tout le monde voit la code review comme un tribunal. Une affaire, un accusé et des jurés.

Une bonne fois pour toutes, il faut arrêter avec cet état d’esprit. La code review ne sert pas à juger. La code review sert à l’amélioration de la qualité de l’application et aux partages des connaissances dans l’équipe. Plus concrètement, on veut trois choses.

Repérer les bugs

L’erreur est humaine. Il faut plusieurs cerveaux pour maximiser les chances d’éviter les problèmes. Même avec plusieurs cerveaux, les bugs sont toujours là. Et ça, c’est bien la preuve que la code review est absolument nécessaire.

Propager la connaissance

J’ai rien contre un certain ownership de bout de système par certaines personnes. Mais ce qu’on veut c’est que tout le monde puisse intervenir un minimum partout. Et la code review, c’est juste parfait, taillé sur mesure pour permettre ça.

Amélioration de la qualité

Dans mon article sur les bonnes équipes de développeurs, je disais que la qualité est un effort d’équipe. Une discipline de groupe, une tradition forte qui devrait être sauvagement défendue par tout le monde. Cet effort passe énormément par la code review.

La code review est censé être un échange de savoir bidirectionnel, une discussion, pas un tribunal correctionnel.

Faites entrer l’accusé

Quand Mr X à vu que la code review m’a été assigné, il a vu rouge.

Il y a une énorme différence entre une code review privée entre deux personnes, et une code review qui devient publique. En particulier quand ça se passe mal.

Et la situation s’est brutalement aggravée quand Mr X nous a montré du doigt auprès de la direction. Je dis nous, car même si je n’avais même pas encore participé, je faisais déjà partie du camp adverse.

En nous montrant du doigt, il traça une ligne dans le sable. À partir de ce moment-là, plus personne ne franchit cette ligne.

C’était trop tard, il n’était plus question de code.

Plaidoirie

Pour éviter ce genre de situation infernale, voilà quelques règles indispensables pour celui qui envoie la code review.

Envoyer du code fonctionnel et testé

Ça semble évident. Ça ne l’est pas. Le problème principal étant la pression mise sur les développeurs au niveau du temps. En sautant quelques principes de base, c’est sûr que ça va plus vite.

Une manière efficace d’y arriver et de toujours se mettre à la place de la personne qui va regarder ta code review. Faire la code review de ton propre code comme si c’était le code de quelqu’un d’autre.

Faire ça -juste ça- propulse ta code review dans le top 1% monde.

Envoyer le moins de code possible

Le moins de code tu enverras, le mieux ça sera. La personne qui va la recevoir va être reconnaissante et du coup efficace. Ça permet également de mieux contrôler les changements apporté à ton application et de limiter les bugs.

Une manière efficace d’y arriver et de travailler en amont pour réduire le scope des tâches.

Si tu vois qu’une tâche va nécessiter énormément de code, insiste pour qu’elle soit découpée en plusieurs tâches.

Envoyer d’abord le contexte

Il n’y a rien de pire que d’envoyer du code à quelqu’un sans aucun contexte. Tu imposes un double travail à ton collègue. Si tu veux des efforts de la personne, tu dois faire des efforts de clarification en amont.

Une manière efficace d’y arriver et de mettre une bonne description du problème que règle ta solution dans la description de ta code review. Pourquoi pas un lien direct vers le ticket, si le ticket est bien renseigné.

Tout ce dont la personne aura besoin pour optimiser son temps de compréhension est précieux.

Prendre du recul

Une code review a pour objectif de challenger ta solution. On va challenger ton code, pas toi personnellement. Les gens qui n’ont pas ce recul sont incapables de faire une code review normalement.

Une manière efficace d’y arriver et de te souvenir que tu n’es pas ton code. Que rien n’est parfait et que chaque critique est une opportunité d’amélioration. Pas une attaque personnelle.

L’incroyable égo des développeurs est fortement mis à l’épreuve ici.

Jurés

Il y a également des règles indispensables à respecter pour celui qui reçoit la code review.

Exiger le minimum

Pour faire ton travail de revue efficacement tu dois mettre certaines barrières à l’entrée. Ces barrières devraient être les mêmes pour tout le monde. Tu veux optimiser l’utilisation de ton temps et de celui qui t’envoie la review.

Si tu ne vois aucun test, aucun contexte, tu la renvoies poliment. Avant de faire la revue, tu checkout la branche et tu lances les tests chez toi. Une fois que tu es sur du sérieux de la code review, tu y consacres du temps.

Poser des questions

Beaucoup de développeurs oublient que les commentaires qu’ils laissent sont lus par un être humain.

La façon dont tu fais tes retours et aussi importante que tes retours eux-mêmes. Si tu es un connard, mais que tu as raison, tu es un connard qui a raison. Et devine quoi ? Personne ne veut travailler avec un connard.

J’ai une manière très efficace pour avoir du tact dans le cadre d’une code review. Il suffit de poser des questions au lieu de donner des ordres.

Concrètement, au lieu de dire « c’est de la merde ta façon de faire ici » tu peux dire « que pense-tu de cette autre manière de faire à la place ? ».

Non seulement tu n’agresses pas ton collègue, mais en plus tu apportes une solution au lieu d’être seulement dans la critique. Crois mois, c’est la botte secrète qui change absolument tout.

En ressenti, tu passes du connard arrogant, à la personne qu’on respecte pour son tact et ces compétences techniques.

En plus, si tu as tort dans ton analyse et que ta remarque était mauvaise : c’était juste une question. Tu pourras dire que tu étais juste curieux et que tu comprends mieux désormais.

Également, celui qui reçoit la code review doit maitriser le côté technique.
Et pour cette partie, il faut commencer par la jurisprudence.

Jurisprudence

Chaque équipe devrait avoir une suite de règle qui régissent comme le code doit être structuré et stylisé dans l’application. Cette jurisprudence est la source de vérité absolue que tout le monde doit respecter. Le rôle de la code review et aussi de faire respecter cette jurisprudence.

Pour te donner un exemple concret, voici la jurisprudence pour tout développeur qui fait du Javascript à Airbnb : https://github.com/airbnb/javascript.

Le même genre de règles pour établir des règles de styles dans ton équipe doit exister.

Si ce n’est pas le cas chez toi, tu devrais le proposer.

Ça mettra fin à tous les débats et les discussions inutiles sur les détails et le style du code. La jurisprudence faisant foi, on peut maintenant s’intéresser au cœur du problème.

Techniquement

Maintenant, techniquement comment tu fais ? Comment tu trouves quoi dire dans une code review ? Comment tu repères du code douteux et comment tu sais quoi proposer comme solution ?

Je ne vais pas te mentir, c’est beaucoup ça aussi qui fait de toi de quelqu’un d’efficace dans le contexte de la code review.

Il y a deux façons d’arriver à cet objectif.

La première est de t’exposer un maximum à ces codes reviews et aux bonnes pratiques de manière générale. Pendant dans années. Apprendre de développeurs plus expérimentés que toi. T’investir dans les code review. En donner, en recevoir ou juste les observer. Tout faire pour participer fortement à ce sujet. A force, tu finiras pas avoir une connaissance et des réflexes affutés.

Mais cette façon là prend du temps. Comment faire pour aller plus vite ?

La seconde manière est un raccourci de la première. C’est ma recommandation du jour : Refactoring : Comment améliorer le code existant.

Ce bouquin est une véritable mine d’or qui m’a appris tout ce que je devais savoir sur l’amélioration du code. Car oui, c’est surtout ça une code review. Une discussion pour améliorer le code qui peut être amélioré.

Les premiers chapitres t’expliquent un peu le principe et les grandes règles du refactoring. C’est intéressant.

Mais le plus intéressant commence au chapitre 3. Ce chapitre est dédié à l’identification formelle du code douteux. Le Saint-Graal, nectar sacré, la source d’efficacité de tes codes reviews.

J’ai dévoré cette partie, je l’ai lut, relue et relue plusieurs fois pour être sûr de bien tout comprendre. J’avais enfin trouvé quoi dire dans les codes reviews que je recevais. Et c’est bien de savoir quoi dire, mais il faut aussi proposer des solutions.

Ça tombe bien, car le dernier chapitre est un catalogue de solution que tu peux apporter à chaque bout de code douteux. Des centaines de pages organisées dans un index pour retrouver facilement ce que tu cherches. J’ai toujours ce bouquin à portée de main.

Cerise sur le gâteau : tout le code pour les exemples dans ce bouquin c’est du Javascript. Aucun prérequis et ca s’adresse aux juniors comme aux seniors.

J’ai personnellement lu la version anglaise, mais tu as une version française disponible aussi !

Dommage et intérêt

Très vite la code review de Mr X avait fait le tour de l’open space.

Les managers ont commencé à poser des questions. Tout le monde se montrait du doigt. La tâche restait dans l’impasse.

Et je dis tout le monde, car je me suis rendu compte que mon collègue qui avait abandonné la review, avait lui-même été assigné dessus par un autre collègue, qui avait abandonné avant lui.

Bien sûr la PR étant désormais publique, tout le monde donnait son avis sur la situation.

Le point culminant étant le jour de la confrontation en personne en plein milieu de l’open space.

L’histoire finie plutôt mal puisque Mr X demanda un transfert dans une autre équipe peu de temps après. Ce fut rapidement accepté et effectif. Il n’était plus question de réparer les dégâts, mais de les limiter.

Pourtant, pour empêcher tout ça, il aurait suffi de faire une seule chose au bon moment.

Quand un drame commence dans une pull request, il faut immédiatement la fermer, et discuter en vrai avec les gens.

Le contexte déshumanisé de la code review exacerbe les réactions, les prises de position et les interprétations. La recette du chaos. Grand destructeur d’équipe.

Que tu le veuilles ou non, une code review est fortement axée sur l’humain et l’ego. Ceux qui ne pensent qu’à la partie technique sont condamnés à produire des drames. Et la situation s’aggrave de façon exponentielle lorsque les deux parties ont cet état d’esprit.

C’est exactement ce qui s’était passer à cette époque.

Cette histoire n’est pas très originale. Elle est déjà arrivée dans beaucoup d’équipes. Et sans un peu de vigilance, ça arrivera dans la tienne.

Épilogue

Une code review n’est pas un tribunal. C’est un échange de connaissances permettant une meilleure qualité et stabilité de ton application. Et évidemment, ça reste une revue technique, mais enlever et/ou piétiner le facteur humain produira un résultat chaotique.