22 - Code review — donner et recevoir du feedback

Ce que tu vas apprendre

• Pourquoi la code review est utile (et ce n'est pas juste la qualité)
• Comment reviewer une PR efficacement
• Comment recevoir du feedback sans le prendre personnellement
• Pourquoi les petites PR obtiennent de meilleures reviews
• Les checks automatiques qui liberent la review humaine
• Le problème du "LGTM" automatique
• Une checklist de review pratique

Prerequisites

Article précédent : 21 - Dette technique

Ma pire experience de code review : un senior qui ecrivait "non" comme seul commentaire sur mes PRs. Pas de contexte, pas de suggestion, pas d'explication. Juste "non". J'ai passe des heures a deviner ce qu'il voulait. Ma confiance a pris un coup. J'ai fini par éviter de soumettre du code quand il etait disponible pour reviewer.

La code review est un des outils les plus puissants d'une équipe. C'est aussi un des plus mal utilises. Bien pratiquee, elle transmet des connaissances, previent les bugs et renforce la confiance. Mal pratiquee, elle ralentit, frustre et demotive.

Pourquoi reviewer le code

La première raison n'est pas la qualité. C'est le partage de connaissances. Quand tu reviewes le code de quelqu'un, tu apprends comment fonctionne une partie du projet que tu ne connais peut-etre pas. Quand quelqu'un review ton code, il comprend ce que tu as construit. La connaissance se diffuse dans l'équipe au lieu de rester dans la tête d'un seul dev.

La deuxieme raison est la détection de bugs. Pas tous les bugs — les tests et le linting en attrapent une partie. Mais un regard humain repère des problèmes que les outils ne voient pas : une logique métier incorrecte, un edge case oublie, une race condition potentielle.

La troisieme raison est la coherence. La review est le moment ou les conventions d'équipe sont transmises et appliquees. Pas par la force — par la discussion.

Comment reviewer une PR

Lis la description d'abord. Une bonne PR a un titre clair et une description qui explique le pourquoi du changement. Si la description est vide, demande-la avant de lire le code. Tu ne peux pas reviewer du code sans comprendre l'intention.

Regarde la structure avant les détails. Commence par les fichiers modifies : quels modules sont touches ? Y a-t-il des nouveaux fichiers ? Des suppressions ? Ca donne une vue d'ensemble avant de plonger dans les lignes.

Concentre-toi sur la logique, pas le style. Le style est le travail du linter et du formatter. Si tu commentes sur l'indentation ou les guillemets, c'est que les outils ne font pas leur boulot. L'article 19 - Linting explique comment automatiser tout ca.

Pose des questions au lieu de dicter. "Pourquoi tu as choisi un Map au lieu d'un objet ici ?" est mieux que "Utilise un objet, pas un Map". La question ouvre la discussion. L'ordre ferme la porte. Peut-etre que le dev a une bonne raison que tu ne vois pas.

typescript// Commentaire de review : mauvais
// "Fais pas ca comme ca"

// Commentaire de review : correct
// "Je vois que tu utilises un Set ici pour deduplicquer les IDs.
//  Est-ce que tu as considere le cas ou les IDs contiennent des objets ?
//  Set compare par reference pour les objets, pas par valeur."

Distingue les blockers des suggestions. Pas tous les commentaires ont le meme poids. Un bug potentiel est un blocker. Une suggestion de nommage est un nice-to-have. Indique clairement la différence :

[blocker] Cette query SQL est vulnerable a l'injection. Utilise des parametres prepares.
[suggestion] Le nom "processData" est assez vague. "transformOrderItems" serait plus explicite.
[question] Est-ce que ce timeout de 5 secondes est suffisant pour les gros fichiers ?
[nit] Typo dans le commentaire ligne 42.

Comment recevoir du feedback

Ne le prends pas personnellement. La review porte sur le code, pas sur toi. C'est plus facile a dire qu'a faire, surtout quand tu as passe 8 heures sur une feature. Mais un commentaire "cette fonction est trop complexe" ne veut pas dire "tu es un mauvais dev".

Explique tes choix. Si quelqu'un questionne une décision, reponds avec le contexte : "J'ai utilise un Map parce que les clés sont des objets complexes et que la comparaison par référencé est ce qu'on veut ici." Ca fait avancer la discussion.

Accepte les bonnes suggestions. Si le reviewer a raison, change le code. Pas de debat d'ego. "Bonne idee, je corrige" est la meilleure réponse possible.

Resiste aux mauvaises suggestions. Si tu n'es pas d'accord et que tu as de bons arguments, dis-le poliment. "Je préféré garder cette approche parce que [raison]. Qu'est-ce que tu en penses ?" Un desaccord technique sain est normal et productif.

La taille de la PR compte

Les etudes de SmartBear sur la code review montrent qu'un revieweur est efficace sur environ 200 a 400 lignes de code. Au-dela, la qualité de la review chute. Sur une PR de 2 000 lignes, le reviewer survole et approuve sans vraiment vérifier. C'est humain.

PR de 50 lignes  -> review de qualite en 10 minutes
PR de 200 lignes -> review correcte en 30 minutes
PR de 500 lignes -> review superficielle en 45 minutes
PR de 2000 lignes -> "LGTM" en 5 minutes

Decoupe tes grosses features en plusieurs PR. La première PR pose la structure (types, interfaces, schemas). La deuxieme ajoute la logique métier. La troisieme connecte a l'API. Chaque PR est comprehensible individuellement.

Ca demande de la discipline. C'est plus facile de tout mettre dans une seule PR. Mais c'est aussi la garantie que personne ne la relira serieusement.

Les checks automatiques avant la review humaine

Le temps du reviewer est precieux. Ne le gaspille pas sur des choses qu'une machine peut vérifier. Avant qu'un humain regarde la PR, la CI doit vérifier :

• Le build compile
• Les tests passent
• Le linting est conforme
• Le formatting est correct
• La couverture de tests n'a pas baisse

Si une de ces verifications echoue, la PR n'est pas prete pour review. C'est automatique, objectif et non-negociable. Le reviewer peut alors se concentrer sur ce que la machine ne sait pas faire : la logique, l'architecture et l'experience utilisateur. Tu peux explorer d'autres pratiques d'équipe sur paltemps.fr.

Le problème du "LGTM"

"LGTM" (Looks Good To Me) est devenu un reflexe. Quelqu'un demande une review, tu ouvres la PR, tu fais defiler, tu ecris "LGTM", tu approuves. Ca prend 30 secondes. Ca ne sert a rien.

Le "LGTM" automatique a deux causes. La première est la surcharge : trop de PRs a reviewer, pas assez de temps. La solution est de limiter le nombre de PRs en cours (WIP limit) et de bloquer du temps pour les reviews dans le calendrier.

La deuxieme est le manque de contexte : le reviewer ne connaît pas le domaine de la PR. La solution est de choisir les reviewers en fonction de leur connaissance du module, pas en round-robin.

Si tu approuves une PR, ajoute au moins un commentaire positif spécifique : "La facon dont tu as gere le retry avec backoff exponentiel est clean." Ca montre que tu as lu le code et ca encourage les bonnes pratiques.

Checklist de review

Voici la checklist que j'utilise quand je review une PR :

markdown## Logique
- [ ] Le code fait ce que la description de la PR annonce
- [ ] Les edge cases sont geres (null, tableau vide, erreurs reseau)
- [ ] Les erreurs sont gerees correctement (pas de catch vide)

## Securite
- [ ] Pas de donnees sensibles en dur (cles API, mots de passe)
- [ ] Les entrees utilisateur sont validees
- [ ] Pas de SQL injection ou XSS possible

## Lisibilite
- [ ] Les noms de variables et fonctions sont explicites
- [ ] La complexite des fonctions est raisonnable
- [ ] Les commentaires expliquent le "pourquoi", pas le "quoi"

## Tests
- [ ] Les nouveaux comportements sont testes
- [ ] Les tests sont lisibles et maintenables
- [ ] Les tests ne sont pas couples a l'implementation

## Architecture
- [ ] Le code respecte les conventions du projet
- [ ] Pas de duplication evitable
- [ ] Les dependances entre modules sont raisonnables

Cette checklist n'est pas un formulaire a remplir mecaniquement. C'est un guide pour ne rien oublier. Avec le temps, les reflexes s'installent et tu n'en as plus besoin.

Sync vs async

Les reviews en asynchrone (commentaires sur la PR) fonctionnent pour la majorite des cas. Mais certaines discussions meritent une conversation en face a face ou en appel : les desaccords architecturaux, les choix de design complexes, les feedbacks delicats.

La regle que j'utilise : si un fil de commentaires dépassé trois aller-retour sans résolution, passe en synchrone. 15 minutes de conversation remplacent une journee de commentaires.

Résumé

• La code review est d'abord un outil de partage de connaissances
• Lis la description avant le code, concentre-toi sur la logique, pose des questions
• Distingue les blockers des suggestions avec des prefixes explicites
• Ne prends pas le feedback personnellement : le code n'est pas toi
• Les petites PRs (200-400 lignes) obtiennent des reviews de meilleure qualité
• Automatise tout ce qu'une machine peut vérifier avant la review humaine
• Le "LGTM" sans commentaire est un signal que le processus de review est casse
• Passe en synchrone apres trois aller-retour sans résolution

Article précédent : 21 - Dette technique

Article suivant : 23 - Glossaire

Sources

• SmartBear, "Best Practices for Code Review" - https://smartbear.com/learn/code-review/best-practices-for-peer-code-review/
• Google Engineering Practices, "How to do a code review" - https://google.github.io/eng-practices/review/reviewer/
• Michaela Greiler, "Code Review Best Practices" - https://www.michaelagreiler.com/code-review-best-practices/