On parle souvent de revue de code ou de peer review dans les bonnes pratiques de développement.
En quoi ca consiste?
L’objectif de la peer revue est la relecture du code par un pair afin de s’assurer de la bonne qualité de code et de la conformité aux règles de développement de l’entreprise.
Quels sont les avantages ?
L’avantage est multiple, d’un côté on s’assure que le code soit qualitatif et répond au besoin, d’un autre côté plusieurs membres de l’équipe de développement s’approprient le code et la compréhension métier associé. Enfin, lire le code produit par un autre développeur permet de monter en compétences.
Qui fait la revue de code?
Idéalement, dans une équipe mature, la première personne disponible s’occupe de la revue. Il faut éviter que ça soit toujours la même personne (généralement le lead dev, ou le dev sénior) qui s’occupe des revues, pour favoriser la propriété collective du code.
Ok et je suis censé faire quoi quand on m’assigne une revue?
Les points essentiels à contrôler dans une revue de codes sont les suivants :
- le code répond il au besoin exprimé dans le ticket/tache à faire ?
- le code est-il bien structuré et rangé au bon endroit ? faut-il le découper ?
- les méthodes sont-elles suffisamment génériques pour être réutilisables ?
- le code est-il bien documenté (swagger pour les api rest, javadoc pour le code, commentaires etc)
- le code est-il bien testé ? est ce que les règles de gestion exprimées par le besoin métier sont bien couvertes par le test? les cas à la marge (NPE/exception etc) sont ils bien testés ?
- le code répond-il au standard de l’entreprise ? (règle de nommage/architecture/pattern de développement/format d’api/règle sonar de couverture de code etc)
Recommandé :
- Se mettre sur la branche en question, exécuter l’application, tester le service impacté, la nouvelle api créée, le correctif ou l’évolution.
Si tous ces points sont au vert, tu peux approuver la merge request et/ou la merger !
Sinon, il faut apporter des commentaires sur les points qui ne sont pas corrects. Généralement cela se fait directement dans gitlab ou github. Tu peux également aller échanger avec la personne en direct pour apporter des précisions mais je te recommande de laisser une trace écrite de ce qui ne va pas.
Partager, comprendre, échanger :
La revue de code est avant tout un moment de partage entre développeurs. Que ce soit au niveau de la compréhension fonctionnelle de chacun, de l’implémentation technique ou de la façon de s’approprier un sujet plus ou moins complexe.
Lorsque l’on regarde le code réalisé par un pair, il est important de rester bienveillant, de comprendre ce qui à été fait, d’expliquer ce qui pourrait être mieux fait ou fait différemment.
La revue de code peut également être collective. Dans ce cas on regarde en séance le code réalisé par un membre de l’équipe, cela permet de détecter plus de défaut et que l’ensemble de l’équipe s’approprie le code. En revanche c’est plus coûteux pour l’entreprise.
Que faire des défauts relevés lors de la revue?
Lorsque la correction est claire et ne fait pas débat ( mauvaise implémentation, non respect des standards etc.. ) l’auteur du code réalise la correction après la revue et on s’assure que les défauts sont bien corrigés avant de merger les développements.
Lorsque le problème repose sur un conflit d’implémentation, par exemple lorsque l’implémentation de la solution est possible de différentes manière, que le codeur et le reviewer ne sont pas en accords sur la solution mise en oeuvre ou bien lorsqu’aucun standard n’est établi concernant la solution mise en œuvre et que plusieurs approches sont possibles, il peut être nécessaire de faire intervenir un tiers ou toute l’équipe pour faciliter la prise de décision.
Enfin, lorsque la correction nécessite de définir un nouveau standard, il est important de s’assurer qu’il soit bien partagé avec tous.
Ok et si je suis junior sur le projet ou la techno ?
Pas de panique !
Techniquement, il se peut que certains concepts, certaines annotations ou certaines façons de coder te soient obscures. Profites en pour te documenter dessus en faisant une recherche google/IA, tu peux aussi demander à ton pair de t’expliquer son code, ca te permettra de progresser.
Fonctionnellement, il se peut que tu ne comprennes pas bien le métier associé au code. Dans ce cas, les tests doivent être clairs sur ce qu’ils testent et refléter le besoin métier, si tu ne comprends pas, c’est que le code n’est pas clair. Tu peux aussi demander à ton pair ou au PO/MOA/MOE de t’expliquer le métier associé.
Quels outils ?
Pour la mise en forme :
Sur le respect du formatage de code, il est important de mettre en place des règles de formatage et de nommage de code.
Les linters :
Les linters permettent de définir des règles nommages de classes / méthodes et autres critères.
Voir JSHint http://jshint.com/?ref=mindsers.blog ou JSLint http://www.jslint.com/?ref=mindsers.blog , checkstyle Java https://checkstyle.sourceforge.io/.
Le formatage du code peut s’exécuter de façon automatique via l’IDE , sur le save, le commit (hook) etc..
- le formatage peut également être mis en place sur la Merge/Pull request sur gitlab ou github. Cela prend la forme d’une tâche dite de “pre-commit”
Pour la revue :
Il existe des plugins que l’on peut ajouter à l’IDE pour faciliter la revue de code, notamment “Gitlab Merge Requests” sur Intellij .
Ce plugin permet de relire commit par commit, ajouter des commentaires et valider des Merge Request directement dans l’IDE
- le rebase / squash commit : Si tu utilise un repo Git (ce que je te souhaite !) il est important de tenir ta branche à jour et de rebase avant d’envoyer ta Merge Request. Le rebase permet d’appliquer tes modifs par dessus la dernière version de la branche, cela permet également d’éviter les conflits pendant le merge.
Voilà avec tout ça, tu es prêt pour faire une bonne revue !
Liens :
- Guide pour la mise en place de checkstyle https://www.baeldung.com/checkstyle-java
- Plugin Intellij Gitlab Merge Requests https://plugins.jetbrains.com/plugin/18689-gitlab-merge-requests
Auteur : Bresson Guillaume / TechLead Backend
Guillaume BRESSON
Développeur enthousiaste depuis plus de 10 ans, j’affectionne particulièrement les sujets backend.
Issu d’une formation d’ingénieur, j’ai commencé ma carrière en intégrant des portails intranet / internet puis je me suis spécialisé sur le développement backend autour de Java/Groovy/Spring.
Mes sujets de prédilection vont de la conception d’api REST à la mise en place de tuyauterie back to back. Je considère que le métier de développeur actuel réside principalement dans l’écriture de tests pertinents.
Mon expérience me permet aujourd’hui de partager les bonnes pratiques autour du développement, des tests (Junit / Mockito /MockMvc) et de l’architecture applicative type DDD (Domain Driven Design).