Revue de code
Examiner le changement que l'utilisateur désigne (un diff, une branche, une PR, ou l'arbre de travail non commité). S'il ne le précise pas, examiner les modifications non commitées, puis la branche courante par rapport à sa branche de base.
Comment revoir
- Comprendre d'abord l'intention. Lire la description de la PR, les messages de commit et le problème lié. Un changement ne peut être erroné que par rapport à ce qu'il essaie d'accomplir.
- Lire l'ensemble du diff, puis le code environnant. Ouvrir les fonctions qui ont changé et leurs appelants. La plupart des vrais bugs se situent à la frontière entre le nouveau code et le code existant.
- Vérifier, dans cet ordre :
- Correction : erreurs de logique, décalage par un, mauvaises conditions, cas
null/vide/erreur non traités, conditions de concurrence, fuites de ressources, propagation d'erreur cassée. - Changements de comportement : API publiques, formats de fichiers, valeurs par défaut ou schémas de base de données qui changent sans migration ni note.
- Sécurité : entrée non fiable atteignant des requêtes, shells, chemins de fichiers, HTML ou désérialiseurs ; secrets dans le code ou les logs ; contrôles d'autorisation manquants.
- Tests : un test échoue-t-il si le changement est annulé ? Les cas limites sont-ils couverts ?
- Lisibilité : noms trompeurs, fonctions qui font deux choses, commentaires qui contredisent le code, code mort.
- Correction : erreurs de logique, décalage par un, mauvaises conditions, cas
- Vérifier avant de signaler. Pour chaque bug suspecté, tracer une entrée concrète qui le déclenche. Abandonner tout ce qui ne peut pas être rendu concret, ou le qualifier de question.
Comment signaler
- Commencer par un verdict en une ligne : prêt à fusionner, fusionner après corrections, ou nécessite une reprise.
- Lister les constats du plus sévère au moins sévère. Pour chacun :
fichier:ligne, ce qui ne va pas, un scénario d'échec concret et une correction suggérée. - Séparer les corrections obligatoires des suggestions et des questions.
- Ne pas alourdir la revue avec des nits de style qu'un formateur ou un linter attraperait, et ne pas reformuler ce que le diff fait.
- Dire ce qui est bon lorsque c'est notable (un test clair, une migration soignée). Limiter à une ligne.