La revue de code est l’habitude au plus fort levier qu’une equipe puisse avoir, et c’est aussi celle qu’on rate le plus souvent. J’ai recu des revues qui ressemblaient a un interrogatoire et des revues qui approuvaient 800 lignes avec un pouce leve en quatre secondes. Les deux sont des echecs. Une bonne revue attrape de vrais problemes, diffuse la connaissance et laisse l’auteur soutenu plutot que juge.
Relisez ce que les humains font bien
Ne gaspillez pas votre attention sur le formatage, l’ordre des imports ou la longueur d’une ligne. Un linter et un formateur automatique s’en chargent, et ils ne se fatiguent jamais. Si votre equipe se dispute sur le style dans les commentaires de PR, vous avez un manque d’outillage, pas un probleme de discipline. Configurez le formateur, validez la config, et passez a autre chose.
Ce qu’une machine ne peut pas verifier, c’est si le code est correct, s’il resout le vrai probleme, et si quelqu’un dans six mois le comprendra. C’est la que va mon attention :
- Est-ce que ca fait ce que la description annonce ?
- Que se passe-t-il aux bords : entree vide, valeurs nulles, acces concurrent, un appel reseau qui pend ?
- Y a-t-il une approche plus simple cachee derriere celle-ci ?
- Le nommage aura-t-il du sens pour quelqu’un qui n’etait pas dans la piece ?
Gardez des changements assez petits pour etre vraiment relus
La limite dure de la qualite de relecture, c’est la taille. La recherche et mon experience disent la meme chose : au-dela de quelques centaines de lignes, la detection de defauts s’effondre parce que les relecteurs survolent. Quand je recois une PR enorme, je demande a l’auteur de la decouper. Une serie de petites PR ciblees recoit un vrai examen sur chacune. C’est aussi pourquoi je rebase et ecrase avec soin avant d’ouvrir une PR, une habitude que j’ai decrite dans les bonnes pratiques Git.
Commentez comme un collegue, pas comme un compilateur
Le ton fait l’essentiel du travail. Le meme point passe completement differemment selon la formulation. Je pose des questions au lieu de rendre des verdicts, et je clarifie quels commentaires sont bloquants et lesquels sont optionnels.
# Au lieu de :
C'est faux.
# Essayez :
Que se passe-t-il ici si items est vide ? Je crois
qu'on depasserait la fin. On pourrait proteger avec
une verification de longueur, ou ce cas est-il
impossible en amont ?
# Et etiquetez les details :
detail : on pourrait inliner, non bloquantMarquer les details comme non bloquants est une petite chose qui retire enormement de friction. L’auteur sait ce qu’il doit corriger pour fusionner par rapport a ce qui n’est que mon gout. Je prends aussi soin de dire quand quelque chose est vraiment bien. Une revue qui n’est que critique entraine les gens a redouter le processus.
Relisez vite et finissez ce que vous commencez
Une PR qui reste sans relecture deux jours bloque une personne et pourrit a mesure que main avance dessous. Je traite les demandes de relecture comme une priorite quasi en haut de ma file, idealement le jour meme. Le cout d’une PR a l’arret s’accumule : l’auteur change de contexte, puis doit tout recharger quand le retour arrive enfin.
Quand je relis, j’essaie de donner tous mes retours en une passe plutot que de distiller des commentaires sur trois tours. Rien n’est plus demoralisant que tout corriger, etre relu, et decouvrir cinq nouveaux commentaires qui etaient visibles depuis le debut.
L’auteur a aussi des devoirs
Les revues vont plus vite quand l’auteur les rend faciles. Avant de demander une relecture, j’ecris une description qui explique ce qui a change et pourquoi, je laisse des commentaires en ligne sur mon propre diff pour pointer ce qui n’est pas evident, et je m’assure que la CI est au vert. Une bonne description peut diviser par deux le temps de relecture, car le relecteur ne reconstitue pas l’intention a partir du diff.
- Enoncez le probleme, pas seulement la solution.
- Signalez tout ce dont vous n’etes pas sur et sur quoi vous voulez un regard.
- Incluez des captures ou un exemple de sortie pour tout ce qui touche l’utilisateur.
Etre en desaccord, puis s’engager
Parfois l’auteur et le relecteur voient simplement les choses autrement. Quand le desaccord porte sur le gout plutot que sur la justesse, je m’en remets a la decision de l’auteur apres avoir exprime mon avis une fois. Trainer une PR sur cinq tours pour une preference stylistique brule un capital de bonne volonte dont vous aurez besoin plus tard. Gardez les positions fermes pour ce qui compte vraiment : la justesse, la securite, et les contrats de donnees qui me tiennent tant a coeur dans le guide de conception d’API REST. Reussissez ceux-la et laissez filer les petites choses.