dans Programmation 101
Comment faire et recevoir des bonnes revues de code
Mon premier article de 2021! J’ai le goût de te souhaiter une année pas si pire… Je ne voudrais pas dire : « Ça peut pas être pire que 2020! » de peur de nous jynxer. Bref, envie de débuter l’année avec un article simple sur les revues de code. Aujourd’hui, j’aimerais te montrer comme comment faire et recevoir des revues de code qui vont être profitables.
À quoi ça sert?
Certains devz craignent les revues de code. C’est un peu comme la police du code, non? Valider que ce que tu as produits, c’est digne d’être dans le code base de l’équipe?
Nope.
Valider le formatage, que tu n’as pas oublié de mettre un espace devant une accolade?
Nope.
S’obstiner pendant 8,172 commentaires à savoir si tu dois VRAIMENT remplacer ta condition ternaire par un if standard pour aller en production?
Nope! C’est beaucoup plus complexe et bénéfique pour une équipe que ça.
Partage de connaissances
Tu prends une tâche et tu la codes. Un pair va prendre connaissance de la tâche, regarder comment ça été implémenté, et apprendre les nouvelles fonctionnalités du système. Ceci augmente beaucoup notre compréhension globale de l’application et notre rapidité à trouver des bogues ou développer des nouvelles fonctionnalités.
Validations organique
Ici, on valide principalement des choses qui ne peuvent pas être couvertes par un linter ou d’autres scripts de validation. Par exemple, s’il existe un service commun pour effectuer une opération, mais que tu t’en crées un nouveau qui fait sensiblement la même chose sans trop savoir que l’autre existait, ça pourrait sortir dans les commentaires d’une revue de code.
Toujours d’un point de vue organique, la revue de code sert aussi à s’assurer que les choses sont aux bons endroits. Il faut valider que les fichiers ont été créés aux bons endroits dans l’arborescence, que les namespaces et / ou packages et / ou modules etc. sont bons et que les fichiers sont bien nommés (c’est bon des édashous).
La nomenclature aussi doit être validée. La revue de code est parfaite pour ça. Si la personne qui fait la revue peine à comprendre, c’est qu’il y a un problème au niveau du découpage du code et de la nomenclature des variables, fonctions, classes, etc.. Ici, j’irais en mode proposition. En se basant sur sa propre compréhension du code, proposer 2 ou 3 nouveaux noms au lieu de simplement demander de renommer.
|
1 2 3 4 5 6 7 8 9 10 11 |
const employees = []; // supposons des données for (let employee of employees) { // faire quelque chose avec employee } // vs const employeeArray = []; // supposons des données for (let currentEmployee of employeeArray) { // faire quelque chose avec currentEmployee } |
Validations algorithmique
Sans nécessairement parler d’algorithmes complexes, on peut ici valider la performance globale du code produit. Par exemple, une boucle asynchrone avec des appels backend à chaque itération peut être extrêmement coûteuse en termes de performance, comme une boucle qui fait un commit à la base de données à chaque itération.
|
1 2 3 4 5 6 7 8 9 |
const collection = []; // supposons des données for (let current of collection) { await service.doSomethingWith(current); } // vs await Promise.all(collection.map(current => service.doSomethingWith(current))); |
Validations de sécurité
Tout dépendant du contexte et des technologies en place, une revue de code peut être aussi pratique pour valider les failles de sécurité les plus courantes, comme le manque de validations backend des variables passées en paramètres, des injections de script ou SQL, de la journalisation de données sensibles, etc.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 |
class Controller { async controllerMethod(fileName) { const fileContent = await fileService.readFileContent(`/path/to/folder/${fileName}`); // faire quelque chose avec le fichier } } // vs class Controller { async controllerMethod(fileName) { if (this.isFileNameInvalid(fileName)) { throw new Error('Invalid file name'); } const fileContent = await fileService.readFileContent(`/path/to/folder/${fileName}`); // faire quelque chose avec le fichier } isFileNameInvalid(fileName) { return fileName == null || fileName.indexOf('..') > -1 || fileName.indexOf('/') > -1 || fileName.indexOf('\\') > -1; } } |
Comment s’assurer d’avoir une bonne revue de code?
Il va de soit que si tu push 50 commits qui contiennent 293 fichiers modifiés, la revue de code va être pénible à faire et tu risques de ne pas avoir les bénéfices escomptés. Il faut donc penser à faire les plus petites pull requests possibles, quitte à demander plusieurs revues de code pour une même tâche.
La meilleure façon d’avoir une certaine qualité au niveau des revues de code est de monter une liste de points à valider. Dans un monde idéal, votre équipe a un gabarit de description de pull request qui contient des cases à cocher pour les points à valider.
De plus, si tu te forces à faire des bonnes revues de code, tu risques d’en recevoir aussi 😉
Quels sont les pièges à éviter?
Comme je me suis amusé à expliquer en introduction, tout ce qui peut être automatisé doit l’être. Vous ne devez pas bloquer une pull request s’il manque un espace devant l’accolade si vous avez un linter qui manque de configuration. Ouvrez une tâche au backlog pour corriger le linter.
Évitez les commentaires qui n’apportent aucune valeur au logiciel. Si vous comprenez le code, mais que vous auriez fait ça autrement, laissez tomber. Si vous préférez un if au lieu d’une condition ternaire, vous devez convaincre l’équipe afin que ça devienne une règle claire.
Faites des phrases claires et neutres dans vos commentaires. Tout dépendant de la personnalité des membres de l’équipe, certains peuvent être plus facilement froissés que d’autres au sujet des commentaires. Ceci devient encore plus vrai lorsque le programmeur sent que le commentaire est injustifié, d’où l’importance d’avoir une liste de validations faite en équipe.
Conclusion
Aujourd’hui, tu as appris ce qu’est une bonne revue de code et les pièges à éviter. Gardez en tête que si votre équipe se munie d’une bonne liste de validations et que les pull requests sont petites, la qualité des revues de code augmentera et votre équipe sera plus performante.
Si tu as aimé, n’hésite pas à partager avec tes collègues et sur les réseaux sociaux 🙂 Notre blogue est gratuit, sans pub, propulsé uniquement par notre passion pour le code.
Cheers!
Commentaires
2 commentaires
Très instructif 🙂
Merci!
Laisser un commentaire