Guide pour faire des revues de code dans votre équipe

Les revues de code sont une pratique qui s’est généralisée ces dernières années, où un ou plusieurs développeurs examinent le nouveau code implémenté par un autre collègue, dans le but de détecter des problèmes de qualité du code, des bugs, des vulnérabilités, de mauvaises pratiques, etc. Cela permet de raccourcir les boucles de rétroaction, ce qui est très bénéfique car plus un problème est découvert tard, plus le coût de sa correction est élevé et plus l’impact potentiel sur l’activité est important.

Le code étant le matériau essentiel à partir duquel le logiciel est construit, et étant créé et maintenu quotidiennement par différents développeurs et équipes, il est vital de disposer de mécanismes garantissant sa qualité. Effectuer de bonnes revues de code, consciencieusement et selon les meilleures pratiques, peut faire la différence entre le succès ou l’échec d’une application et, en fin de compte, d’une entreprise. Par conséquent, l’une des premières étapes recommandées pour adopter les revues de code est de disposer d’un guide pratique pour fournir des orientations et une direction à toute l’équipe. Cet article est destiné à servir de guide initial que les équipes peuvent utiliser et adapter à leur contexte.

Les fondamentaux d’un bon code

Avant d’entrer dans les détails, je trouve intéressant de partager ce document qui, bien qu’il s’appelle Zen of Python, est un bon guide pour tout langage.

Le beau est mieux que le laid.
L’explicite est mieux que l’implicite.
Le simple est mieux que le complexe.
Le complexe est mieux que le compliqué.
Le plat est mieux que l’imbriqué.
L’aéré est mieux que le dense.
La lisibilité est importante.
Les cas spéciaux ne sont pas assez spéciaux pour enfreindre les règles.
Bien que la praticité l’emporte sur la pureté.
Les erreurs ne devraient jamais passer silencieusement.
Sauf si elles sont explicitement réduites au silence.
Face à l’ambiguïté, rejetez la tentation de deviner.
Il devrait y avoir une – et de préférence une seule – façon évidente de le faire.
Bien que cette façon ne soit pas évidente au premier abord, sauf si vous êtes Néerlandais.
Maintenant est mieux que jamais.
Bien que jamais soit souvent mieux que *tout de suite* maintenant.
Si l’implémentation est difficile à expliquer, c’est une mauvaise idée.
Si l’implémentation est facile à expliquer, c’est peut-être une bonne idée.
Les espaces de noms sont une formidable bonne idée – faisons-en davantage !

Source : https://peps.python.org/pep-0020/

Objectifs des revues de code

(Style) Détecter les divergences par rapport au style défini dans les guides de style (il est conseillé de définir des guides de style pour le code de notre entreprise) et dans le code du projet.
(Architecture) Vérifier si l’implémentation suit l’architecture du code.
(Modularité) Vérifier si le nouveau code est trop fortement couplé. Vérifier que les composants ont de la cohésion, ont un objectif unique.
(Composition et héritage) Examiner si l’héritage et la composition sont utilisés correctement. Et si la priorité est donnée à la composition plutôt qu’à l’héritage.
(APIs) Si des APIs sont créées ou étendues, vérifier qu’elles sont cohérentes et propres.
(Lisibilité) Vérifier que le code est facile à comprendre, à suivre et à maintenir.
(Testabilité) Vérifier que le code dispose de tests unitaires et de tests automatiques qui testent la nouvelle fonctionnalité.
(Prévention des bugs) Détecter les bugs possibles et/ou les cas non couverts par le code.
(Partage des connaissances) Familiariser le reste des développeurs avec les changements effectués dans le code. Cela permet de mieux distribuer les connaissances dans l’équipe.
(Formation) Transférer les connaissances des programmeurs seniors vers les nouveaux arrivants et les juniors.
(Sécurité) Vérifier que le code n’expose pas d’identifiants de quelque nature que ce soit et ne présente pas de failles de sécurité évidentes.

Poser les bases.

L’objectif de la revue n’est pas de critiquer le travail des collègues, mais de l’examiner afin de l’améliorer ensemble, de détecter d’éventuels oublis ou bugs, et d’apprendre grâce au retour d’information. Les commentaires doivent toujours être respectueux et doivent être reçus de manière ouverte, en remerciant notre collègue pour le temps qu’il/elle a consacré à la revue de notre code. La revue ne doit jamais être abordée comme une compétition entre adversaires, mais comme un travail d’équipe.

Quel que soit notre niveau de séniorité en tant que développeurs, nous sommes tous humains et nous faisons tous des erreurs. Un bon développeur, aussi senior soit-il, est toujours ouvert au retour d’information, même s’il vient d’un développeur plus junior.

Étapes à suivre.

Pré-revue

Ouvrez la Pull Request/Merge Request (ou PR / MR) et lisez la description. Si vous manquez encore de contexte pour le changement, ouvrez le ticket dans votre logiciel de gestion de projet et lisez-le. Il est important de lire les critères d’acceptation, pour essayer de détecter si le code peut tous les satisfaire ou non **(bien sûr, le but de la revue de code n’est pas de tester, mais dans de nombreux cas, on peut détecter des critères d’acceptation non couverts simplement en regardant le code). **
Vérifiez que la branche de base est la bonne. Parfois, par erreur, on peut créer une PR contre main, alors qu’on voulait l’intégrer dans une autre branche. Si ce n’est pas le cas, signalez-le et arrêtez la révision. Car si la branche de base n’est pas la bonne, nous pourrions être en train de réviser des changements déjà révisés.

Revue

Commencez par examiner la liste des fichiers modifiés/créés/supprimés. Vérifiez que l’architecture des fichiers et des modules est cohérente avec les directives existantes et les meilleures pratiques (le cas échéant). Vérifiez que les noms suivent la convention de nommage.
Si le code a des tests unitaires (recommandé), commencer par les tests unitaires peut être un bon moyen de comprendre si tous les critères d’acceptation sont couverts ou non. C’est aussi un bon moyen de comprendre le code de l’extérieur. Les tests unitaires doivent couvrir non seulement le chemin nominal, mais aussi les cas limites et les cas d’erreur.
Vérifiez que le style du code suit le guide de style (le cas échéant), sinon, vérifiez qu’il est uniforme avec le code existant. Vérifiez les noms des variables, classes, méthodes, etc.
Vérifiez que l’architecture du nouveau code respecte l’architecture existante. Par exemple : les nouvelles classes sont cohérentes avec les classes existantes. La façon dont le code est organisé dans les fichiers suit les patterns existants.
Vérifiez que le code est faiblement couplé et a de la cohésion. Les classes et les méthodes doivent avoir un objectif unique. Un exemple courant de fort couplage est lorsque l’accès au moteur de base de données pour les requêtes est dispersé dans tout le code. Cela conduit à un fort couplage au moteur de base de données spécifique, rendant difficile son remplacement futur, par exemple.
Vérifiez qu’aucun code n’a été dupliqué. DRY (Don’t Repeat Yourself)
Vérifiez que le code est facile à comprendre. Si nous voyons du code compliqué, nous devrions ajouter un commentaire à la PR, pour que le développeur essaie de le simplifier et/ou d’ajouter des commentaires pour qu’il puisse être compris.
1. Vérifiez que des nombres magiques ou des littéraux ne sont pas utilisés, ce qui rend le code moins compréhensible et moins maintenable. Suggérez l’utilisation de constantes avec un nom auto-explicatif.
Si une API a été modifiée, vérifiez que les changements sont cohérents avec le reste de l’API, et vérifiez que la documentation a été mise à jour.
L’idée est de revoir le nouveau code, mais parfois pour bien le comprendre, nous aurons besoin de lire un peu le contexte, le code déjà existant. Parfois, on peut voir que le changement est incomplet ou incorrect en regardant le contexte.
Vérifiez que le code n’inclut pas de mots de passe, d’identifiants ou d’informations sensibles de quelque nature que ce soit. Si c’est le cas, ces informations sont déjà dans le dépôt et pourraient être compromises, envisagez de changer les identifiants.
Soyez attentif à tout problème de sécurité évident à l’oeil nu. Par exemple, les injections SQL (utiliser une entrée utilisateur directement dans une requête SQL).
Examinez si les changements doivent se refléter dans la documentation du dépôt (par exemple, dans le README), et vérifiez si elle a été mise à jour et est correctement comprise. Par exemple, si la façon de lancer le projet localement a été modifiée, elle doit être clairement mise à jour dans le README.

Post-revue

Dans le cas où nous avons fait des commentaires, soyez attentif à la réponse éventuelle et aux changements dans la PR, pour réviser à nouveau.
Si nous n’avons pas fait de commentaires et que nous trouvons que c’est bien. Indiquez-le dans la PR et acceptez-la. Ici, selon les règles que nous avons définies dans notre équipe, il peut être nécessaire que d’autres personnes la révisent et suivent un processus spécifique pour fusionner la PR.

Notes finales

Ce guide sert de référence générique pour effectuer des revues de code de manière uniforme au sein d’une équipe/organisation. L’idée est que chaque équipe/organisation puisse le personnaliser et l’étendre à son cas particulier, et travailler continuellement pour l’améliorer et l’adapter à ses circonstances.

En plus de toutes ces vérifications manuelles, il est fortement recommandé d’intégrer des outils d’analyse statique de code, des linters et des règles de formatage dans l’IDE, entre autres, dans le processus. Ceux-ci nous permettront de détecter des problèmes, rapidement et à moindre coût, même avant de créer la PR pour demander la revue de code.

Et bien sûr, vos commentaires sont toujours les bienvenus.