Tu peux valider ma pullrequest ?

Illustration par Julien Carreaud.

La revue de code fait partie de la boîte à outils crafts(wo)manship à disposition des développeurs, il n’y a plus besoin de la présenter, tout le monde s’accorde à dire qu’elle est indispensable. Son objectif principal est de trouver des défauts et de les résoudre, mais elle facilite aussi l’apprentissage, le partage, et l’amélioration du code en général.

La revue de code asynchrone , la solution miracle ?

Les règles de la revue de code asynchrone sont assez simples, un développeur ne peut généralement pas approuver son code seul. Il doit avoir fait relire son code par a minima deux de ses pairs. Le code doit également respecter les standards de qualité de code (intégration continue, tests unitaires).

Les pull requests ou PR ont aussi un énorme avantage sur les revues de code collectives, il n’y a plus besoin de trouver un créneau dans l’agenda en espérant avoir trouvé assez de développeurs disponibles. Ou même rechercher une salle avec tout le matériel nécessaire (écran, connectiques, système audio/vidéo de qualité). Les développeurs sur un site géographique différent ou en télétravail ne sont plus mis de coté. Désormais les développeurs sont chacun devant leur poste de travail. Pour ces raisons, cette pratique est désormais largement plébiscitée.

Après plusieurs années de pratiques de revue de code asynchrone, je fais le constat que bien que très pratique, cette solution n’est pas vraiment la solution miracle.

Première constatation, avec le temps, il arrive que dans certaines équipes la revue de code asynchrone se transforme en simple revue de “validation”.
Vous avez surement constaté dans votre équipe l’émergence d’un nouveau profil de développeur : le “sniper reviewer”. La PR est en ligne depuis quelques minutes, et pourtant elle est déjà validée et sans un commentaire. Ce type de profil doit vous mettre en alerte : un malaise s’est installé dans votre équipe.

Seconde constatation, qui pour moi rend la revue de code asynchrone moins efficace, est qu’elle se transforme en pullrequest synchrone.
En effet, qui n’a jamais prononcé ou écrit ces mots ?

Tu peux me valider ma PR, c’est pas grand chose, juste deux ou trois lignes de codes. T’inquiètes c’est rapide, je dois livrer en recette.

Cette simple phrase a pourtant une lourde conséquence :
Le développeur à qui cette phrase est adressée va alors se mettre dans un état d’esprit qui n’est pas propice à trouver des améliorations dans le code. Ni même à poser la moindre question.

Comment est-on arrivé à cette situation?

• Il est occupé : peut être que le développeur était sur une tâche complexe, nécessitant du calme et de la concentration, il n’a donc que très peu de temps à vous accorder. Il va survoler rapidement la PR pour vite retourner sur sa tâche.
• Évolutions mineures : Ce ne sont que deux ou trois lignes de code, un regard rapide sur la modification. Il n’y a peut être pas de gros défauts mais on ne regarde pas le code dans son ensemble, peut être que le code qui vient d’être rajouté existait déjà quelque part ailleurs.
• Vous lui mettez la pression : comme vous lui avez dit, s’il ne valide pas la PR, vous ne pourrez pas l’installer en recette, il y a donc urgence, pas le temps de laisser une remarque de clean code, celle-ci attendra. Le développeur se sent obligé de valider.
• Valider ou rien : c’est un fait, il doit la valider, l’objectif n’est donc pas de trouver des défauts. Son objectif vous lui avez clairement dit, c’est de valider la PR. Il passera donc logiquement moins de temps à trouver des défauts.
• Les commentaires : Laisser un commentaire peut être perçu comme une attaque. N’oubliez pas que la personne qui laisse son commentaire n’est pas en face de nous au moment ou elle le rédige. Certains commentaires ont du mal à passer à l’écrit alors qu’ils seraient perçus comme inoffensif à l’oral. Vous n’avez pas connaissance du sens et de l’intonation, vous n’êtes donc pas en capacité de savoir si la personne a été bienveillante ou non.

Toutes ces raisons influencent sur la qualité de relecture du code, et donc la capacité à trouver des défauts. La grande majorité de vos relectures de code doit se faire dans un climat propice à la relecture.

Comment (re)créer un climat plus propice ?

Illustration Geek & Poke

• Placer du contexte : Il n’y a rien de plus désagréable que de relire du code modifié ou ajouté sans en connaitre le pourquoi. Via Azure Devops / GitHub, vous pouvez ajouter une description, qui aujourd’hui est trop souvent oubliée, un lien vers une carte du Kanban, des captures d’écrans. On doit en quelques secondes savoir ce que fait votre code.
• Mettre en place un rituel : Dans les faits, il n’y a que très peu de revues de code asynchrones qui sont réellement urgentes. C’est plutôt, nous, qui la rendons urgente car plus vite validée, moins de retours seront soumis. Pour éviter cela, vous pouvez mettre en place un ou plusieurs rituels. Par exemple, le matin avant vos rituels agiles vous pouvez consacrer ce temps à la relecture de pullrequest. Ou avant de commencer une nouvelle tâche. Ce rituel doit durer 30 minutes maximum. Les développeurs ne seront plus dérangés au milieu d’une tâche et pourront se consacrer pleinement à la lecture de votre code.
• Utiliser un outil : Bannissez de vos conversations Teams/Slack les liens vers vos PRs. Utilisez un outil pour lister les pullrequest de votre équipe. À titre d’exemple, nous utilisons un outil développé en interne : Skizzle.
  Grâce à cet outil, les développeurs visualisent l’ensemble des pullrequests en attente de relecture. Plus besoin de leur envoyer un message.
• Respecter un standard de code : Pour être efficace, vous devez suivre un standard de code, en mettant en place un standard on se recentre sur la recherche des défauts, on ne va pas discuter sur le format du langage car celui-ci a été défini dans le standard. Pour qu’un standard soit adopté par l’ensemble des développeurs, celui-ci doit être défini en équipe et mis à jour régulièrement.
• Prendre le temps : L’objectif est de livrer de nouvelles fonctionnalités mais ne laissez pas le delivery prendre l’ascendant sur la qualité de votre code. N’oubliez pas que notre quotidien de développeur est de lire du code, alors soignez le.
• Reprenez confiance en vous : Ce n’est pas parce que vous avez reçu 20 commentaires que votre PR est mauvaise. Justement elle suscite un échange, et c’est très bien, c’est le but. Un commentaire ne veut pas forcement dire qu’il doit absolument être corrigé, vous pouvez échanger avec vos pairs pour trouver la solution la plus adaptée.

Pour finir, dois-je continuer les pullrequests ?

La réponse est OUI mais pour garantir un niveau de qualité sur vos revues de codes, elles ne doivent pas excéder 150 à 200 lignes de code et/ou une vingtaine de fichiers au maximum. Au delà de cette limite, les développeurs sont noyés dans la masse d’informations que vous leur mettez à disposition, et ils risquent de survoler vos modifications.

Les modifications apportées à votre code dépassent ce nombre ?
Vous n’avez pas le choix, votre PR modifie énormément de fichiers. Prenez alors le réflexe de compléter la pullrequest avec une revue collective. Aujourd’hui avec des outils comme Teams, Slack ou Live Share, vous pouvez partager votre écran très facilement, la distance n’est plus un problème.

N’oubliez pas que les remarques sur votre code sont là pour l’améliorer et non pour critiquer votre travail. La critique n’a pas sa place dans la revue de code et vous devez toujours respecter ce principe : “Dur avec le code, doux avec les gens”. La bienveillance avant tout.

En conclusion, dans la pratique agile, on vous dit souvent que pour être prise en compte une User Story doit être “INVEST”.
Désormais, pour être relue, je vous propose d’y appliquer un nouveau principe à vos pullrequests : “CGIDC”

• Context: Présenter rapidement l’origine de votre pullrequest. Pourquoi ne pas mettre un lien vers votre user story par exemple?
• Goal: Ajouter dans la description une ligne pour décrire son but
• Implementation Details: Identifier les fichiers sur lesquels vous voulez attirer le regard du relecteur sur un choix technique.
• Comment: Laisser des commentaires pour guider les relecteurs.

PS: Ce principe est une proposition personnelle et n’existe pas — encore —.

Merci aux reviewers Jérôme, Romain, Aurélien & Antoine pour leurs feedback et un immense merci à Julien pour les illustrations.