Revue de Code : la Relecture qui Garde l'Architecture Honnête

Introduction

Une pull request attend une revue depuis trois jours. L'auteur relance une fois, deux fois, puis laisse tomber et enchaîne sur autre chose. Pendant ce temps, à deux bureaux de là, une autre pull request vient d'être approuvée en moins de trente secondes : un « LGTM » poli, aucun commentaire, et un bug évident — une boucle qui ne gère pas le cas où le panier est vide — file tout droit vers la production.

Ces deux pull requests ont un point commun : dans les deux cas, la revue de code n'a servi à rien. Trop lente, et plus personne n'avance. Trop rapide, et personne n'a vraiment regardé. Entre les deux existe une revue qui fait son travail : celle qui ressemble moins à un contrôle qu'à une relecture.

Pourquoi ça compte

On présente souvent la revue de code comme une formalité administrative avant de merger, ou comme un examen que l'auteur doit réussir. Elle sert en réalité un objectif plus précis : c'est, la plupart du temps, le seul moment où quelqu'un d'autre que la personne qui a écrit le code va le regarder avant qu'il ne devienne la réalité de toute l'équipe. Une fois mergé, ce code n'appartient plus vraiment à son auteur : il appartient à quiconque devra le lire, le modifier ou le déboguer dans six mois, souvent une autre personne, parfois l'auteur lui-même qui aura tout oublié du contexte.

Une bonne revue ne cherche donc pas des fautes de style. Elle vérifie qu'un deuxième regard humain a eu l'occasion de dire « attends, et si... » avant que le code ne soit difficile à défaire. C'est une vérification que l'outillage, aussi bon soit-il, ne sait pas faire à lui seul : un linter valide une syntaxe, pas une intention.

Le point de vue de l'auteur de la PR

La qualité d'une revue commence avant même qu'un relecteur n'ouvre l'onglet. Une pull request de deux mille lignes touchant douze fichiers ne se relit pas vraiment : elle s'approuve par lassitude, ou elle bloque l'équipe pendant une semaine. Une pull request de cent lignes avec une intention claire, elle, peut être réellement lue.

La différence ne tient pas qu'à la taille. Elle tient aussi à ce que l'auteur donne à voir. Comparez ces deux descriptions de pull request :

Avant — Fix bug ticket #482

Après — Corrige le calcul de remise pour les paniers vides : computeDiscount retournait NaN au lieu de 0 quand le panier n'avait aucun article, ce qui cassait l'affichage du total sur la page de paiement. Ajoute un test qui couvre ce cas.

La première description ne dit rien : le relecteur doit reconstruire seul le contexte, l'intention, et ce qui a été testé. La seconde fait déjà la moitié du travail de la revue — elle indique où regarder et pourquoi.

Se relire soi-même avant de publier une pull request, en essayant de se regarder comme un étranger à son propre code, change aussi beaucoup de choses. C'est souvent à ce moment-là, et pas pendant l'écriture, qu'on repère la variable mal nommée ou le commentaire devenu obsolète.

Le point de vue du relecteur

Relire du code ne consiste pas à vérifier que les accolades sont bien alignées — l'outillage s'en charge très bien tout seul. Une revue utile cherche des choses qu'aucun linter ne voit : est-ce que cette dépendance a du sens ici ? Est-ce que ce nom de méthode dit ce qu'elle fait vraiment ? Est-ce que ce cas limite a été pensé ?

Prenons un exemple concret. Dans une pull request, un service métier se met soudain à appeler un repository directement :

public class DiscountService {
  private final ProductJpaRepository productRepository;

  public BigDecimal computeDiscount(Cart cart) {
      var product = productRepository.findById(cart.getProductId());
      // ...
  }

}

Rien ici ne casse la compilation. Les tests unitaires du service passent probablement. Et pourtant, un relecteur qui connaît l'architecture du projet devrait tiquer immédiatement : le domaine vient de se mettre à dépendre directement de la couche de persistance. C'est exactement le genre de dérive qu'un test ArchUnit finira par détecter en CI. Mais la revue de code, elle, peut la stopper avant même que la pull request ne soit mergée, avec une explication à l'appui plutôt qu'un test qui échoue froidement quelques heures plus tard.

Toutes les remarques ne se valent pas non plus, et les confondre décourage autant l'auteur qu'un relecteur trop silencieux. Une dépendance qui casse l'architecture est bloquante. Un nom de variable qu'on aurait choisi différemment est une suggestion. Mélanger les deux dans le même ton — ou pire, dans le même commentaire — oblige l'auteur à deviner ce qui compte vraiment. Comparez :

« Ce n'est pas comme ça qu'il faut faire. »

« Ce service métier appelle directement ProductJpaRepository — ça introduit une dépendance du domaine vers l'infrastructure. Tu peux passer par le port ProductRepository déjà défini dans le domaine, comme dans OrderService ? »

Le premier commentaire ferme la discussion. Le second l'ouvre : il nomme le problème, explique pourquoi il compte, et propose une piste. C'est la différence entre corriger quelqu'un et l'aider à corriger son code.

Intégration progressive dans une équipe

Instaurer une culture de revue qui fonctionne ne se décrète pas en une réunion. Une progression par étapes tient mieux dans la durée :

1. Fixer une taille cible pour les pull requests : quelques centaines de lignes maximum, un sujet à la fois. Une grosse fonctionnalité se découpe en plusieurs PR plus faciles à relire.
2. Convenir d'un délai de réponse : par exemple, une première relecture sous 24 heures ouvrées, pour qu'une pull request ne meure jamais d'attente.
3. Distinguer explicitement bloquant et suggestion : certaines équipes préfixent leurs commentaires (bloquant:, nit:) pour lever toute ambiguïté sur ce qui empêche réellement le merge.
4. Documenter les règles d'architecture qui reviennent souvent : comme évoqué dans l'article précédent, transformer une remarque répétée en règle ArchUnit évite de la reformuler à chaque revue.
5. Faire tourner les relecteurs : ne pas laisser la revue reposer sur une seule personne qui « connaît le code », au risque de créer un goulot d'étranglement et un point de défaillance unique dans la connaissance de l'équipe.

Aucune de ces étapes n'exige un big bang : elles s'ajoutent une à une, au rythme où l'équipe les adopte réellement.

La revue comme culture, pas comme surveillance

Une équipe qui prend soin de ses revues de code ne le fait pas par bureaucratie. Elle construit, pull request après pull request, une mémoire collective de pourquoi le code est fait comme il est fait, bien au-delà de ce qu'une documentation à jour parviendrait à capturer. Un développeur qui rejoint l'équipe apprend souvent autant en lisant les commentaires de revue des six derniers mois qu'en lisant le code lui-même.

Dans une démarche de craft, la revue de code est l'endroit où la qualité individuelle devient qualité collective. Elle ne remplace ni les tests automatisés ni les règles d'architecture : elle ajoute ce qu'aucun outil ne sait faire, un regard humain qui comprend l'intention derrière le code, pas seulement sa syntaxe.

Ce qu'il manquait vraiment

Revenons à nos deux pull requests du départ. Celle qui traîne trois jours a surtout besoin d'un contexte plus clair et d'une taille plus raisonnable, pas de relecteurs plus consciencieux. Celle approuvée en trente secondes avait surtout besoin d'un relecteur qui prenne le temps de vraiment lire. Une bonne revue de code se situe entre les deux : une relecture au sens plein du terme, celle qui donne au code une chance d'être meilleur avant de devenir, discrètement, le problème de quelqu'un d'autre.