Les bons développeurs sont flemmards, c’est un fait connu. Après avoir développé une fonctionnalité il est courant de vouloir s’en débarrasser le plus vite possible en la fusionnant avec la branche principale du projet. Malheureusement, un tel comportement apporte de nombreux problèmes à la longue : bugs, parties du code incompréhensibles, doublons, choix d’implémentations douteux, tests oubliés qui font que votre projet acquiert de la dette technique et devient de plus en plus difficile à maintenir et à faire évoluer.
C’est pourquoi avant de fusionner votre travail, il est important de vérifier qu’il est vraiment terminé, pour cela vous devez compter sur votre équipe !
En principe, lorsque vous partagez votre travail avec les autres développeurs vous faites une Pull-Request. Des plateformes comme Github ou Gitlab (entre autre) permettent à votre équipe de voir tous les ajouts et changements que vous avez fait au sein de votre branche, mais également de commenter chacune des lignes de code… C’est l’endroit idéal pour faire votre revue de code, en anglais Code Review (CR) !
Bon alors, qui relit le code ? En principe, si vous êtes une équipe : tous les développeurs ! En général, je pense que l’accord d’un développeur expérimenté suffit pour les petites tâches ou la correction d’un bug simple, pour tout le reste, il est préférable d’attendre l’avis d’au moins deux ou trois développeurs. Il est important de vérifier absolument tout, même un code court et simple, ainsi on n’oublie rien, la CR doit devenir une habitude intégrée dans votre workflow de développement et pas une exception.
Pour indiquer à l’équipe que votre Pull Request attend une CR, je recommande d’y ajouter un label « Needs CR ». Tous les développeurs peuvent ensuite consacrer un peu de temps de leur journée, (après chaque pause par exemple) à faire les différentes relectures de code.
Vous vous demandez ce qu’on peut vérifier ? Personnellement je suis la check-list suivante :
– Est-ce que ça fonctionne ? La logique est-elle correcte, et est-ce bien ce qu’on attendait ?
– Est-il facile à comprendre ? (Correctement structuré, le choix des noms est bon, …)
– Y a-t-il des parties redondantes ou en double ?
– Est-il suffisamment modulaire, réutilisable, évite-t-il les variables globales ?
– Du code peut-il être remplacé par des fonctions issues des librairies ?
– Est-il optimisé ?
– Est-il propre ? (Ne comporte pas de code commenté, de logs superflus, de fonctions de debug oubliées…)
– Respect-il les conventions de codage ? (Pour automatiser cette étape, je conseille d’utiliser des linters comme jshint, phpcs)
– L’API est-elle commentée ?
– Les éventuels comportements inhabituels sont-ils commentés ?
– Y a-t-il du code incomplet ? Si oui, est-il indiqué par un marqueur approprié comme « TODO » ?
– Les tests existent-ils et couvrent-ils suffisamment les fonctionnalités ? (Evidemment, il faut commencer par vérifier que les tests passent !)
– Les tests unitaires sont compréhensibles et vérifient suffisamment le code ?
Si vous avez répondu « Oui » à toutes ces questions alors vous pouvez valider la Code Review !
« Mais qui a bien pu ajouter cette classe chelou ??? »
En adoptant la revue du code au sein de votre équipe, la qualité du code de vos développements va être améliorée et chacun fera plus attention à ce qu’il écrit pour éviter de se prendre des non-validations ! De plus chacun sera plus au courant des évolutions et des ajouts au sein du projet en lisant le code des copains.
« Ça sert à quoi ce truc-là RabbitMQ ??? »
La meilleure façon d’apprendre à coder c’est de lire du code, ainsi chacun est susceptible d’apprendre des choses à chaque CR. Pour les nouveaux membres cela facilite grandement l’intégration dans l’équipe et la compréhension des projets.
« Hey Jean-Mimi comment on fait une boucle for ??? »
Les commentaires sont archivés, vous pouvez ainsi retourner regarder les commentaires écris par un copain développeur sur n’importe quelle CR pour répondre à un problème sans avoir à le déranger !
« Le stagiaire a encore tout fait bugger… »
Tout le monde est responsable du code qui a été fusionné, il n’y a plus de coupable de bugs, enfin tout le monde l’est !
Le but n’est pas de juger l’autre, mais d’aider chacun à écrire le meilleur code possible pour conserver ensemble l’intégrité du projet. S’améliorer doit être un objectif partagé par tous au sein de l’équipe, sans quoi les CRs n’auront pas l’effet escompté.
Les CRs doivent être faites rapidement pour éviter la frustration, les retours sont plus simples à corriger pour la personne qui a soumis le code quand celui-ci est encore frais dans sa tête.
La revue code c’est une chose, mais je vous encourage à faire également une UX Review et une QA Review avant de vraiment pouvoir considérer une tâche « terminée ».
Cet article a été inspiré par la lecture de celui-ci et celui-là. Des idées à rajouter dans la liste, ou un avis à donner ? N’hésitez pas à commenter !