L'importance d'être un réviseur de code

Un cadre pour donner un feedback sur le code des autres, par Carlo Sales

« Le feedback est comme un steak - s'il est trop cru, personne ne l'aime, mais s'il est trop cuit, il est difficile à avaler » (ChatGPT)

Réviser régulièrement le code d'autrui peut améliorer vos compétences et donner un coup de pouce à votre carrière. Vous pouvez aider les autres à se développer et apporter de la valeur à l'entreprise avec laquelle vous travaillez.

Dans cet article, nous verrons les avantages de la révision de code dans une pull request (PR), et quelques conseils sur l'approche à suivre pour le faire, comme l'interaction avec d'autres collègues.


Définitions

• Revue de code : le processus qui consiste à donner un feedback écrit à la branche d'un auteur.
• Pull Request (demande de tirage) : le terme GitHub utilisé pour indiquer les différences entre une nouvelle branche et la branche principale, où vous pouvez laisser vos commentaires.

Les revues de code ? N'est-ce pas du passé ?

Si vous êtes un développeur Agile, il se peut que vous remettiez en question la pratique de la revue de code. Par exemple, vos observations peuvent être les suivantes :

• Si une activité cohérente de programmation en binôme est mise en place, il n'y a pas besoin d'un processus formel de revue de code, où un « étranger » vient juger votre code, souvent sans aucun contexte, après tout le travail que vous avez fait et le temps que vous avez passé.
• L'exactitude du code aurait déjà dû être évaluée en utilisant une approche TDD.
• La syntaxe et le style peuvent être automatiquement vérifiés par un linter.

Ces considérations sont certainement valables. Cependant :

Avantages

Faire des revues de code peut apporter beaucoup d'opportunités si vous arrêtez de les voir comme un devoir imposé, ou une tâche ennuyeuse.

Pour le démontrer, j'ai créé un cadre qui peut aider à donner un feedback sur une demande de tirage d'un collègue.

W3H : Why, What, When, How (Pourquoi, Quoi, Quand, Comment)

Même si je ne suis pas un grand fan des acronymes, j'ai inventé celui de W3H pour résumer les questions importantes que vous pouvez vous poser lorsque vous abordez le code de quelqu'un d'autre.

Why (Pourquoi)

La réalisation de revues de code peut être imposée par les processus CI/CD de votre entreprise, de sorte que la réponse à la question « pourquoi » pourrait être « parce que mon organisation m'a dit de le faire ».

Cependant, il est bénéfique de se demander : « Pourquoi est-ce considéré comme si important ? », ou “Pourquoi devrais-je faire des revues de code de manière proactive ?”.

Tout d'abord, il s'agit d'une opportunité d'apprentissage pour le réviseur et l'auteur. L'auteur peut obtenir des conseils et des suggestions qui peuvent améliorer rapidement ses compétences. Le réviseur peut apprendre de nouvelles astuces et de nouveaux idiomes du langage utilisé.

En outre, en tant que développeur, vous pouvez :

1. Comprendre plus rapidement une nouvelle base de code.
2. Créer un réseau, surtout si les équipes sont géographiquement éloignées.
3. Repérer les chevauchements et les doubles emplois potentiels dans le travail d'autres équipes.
4. Appliquer les meilleures pratiques dans la base de code.
5. Être reconnu comme un expert et une personne compétente par les autres ingénieurs et les responsables.
6. Renforcez vos compétences en matière de communication (nous aborderons ce point dans la section « Comment »).
7. Vous faire des amis !

What (Quoi)

Qu'est-ce qu'un code « suffisamment bon » ?

Avant d'écrire des commentaires sur un code écrit par quelqu'un d'autre, vous devez être clair sur les exigences minimales qu'un bon code devrait avoir.

Voici quelques règles générales :

1. Il ne devrait pas y avoir d'erreurs évidentes, comme des fautes de frappe dans les noms, ou une indentation non standard. Je fais généralement ce genre de vérification pour m'échauffer lorsque je regarde une PR sur un module que je ne connais pas bien.
2. Le code doit être structuré et facile à comprendre, même s'il accomplit des tâches complexes.
3. Il ne doit pas y avoir de problèmes majeurs de performance, par exemple une liste lue plusieurs fois alors qu'elle pourrait l'être en un seul cycle. Ceci est particulièrement vrai pour les applications mobiles où le risque est de consommer inutilement de la batterie.
4. Une PR ne doit modifier que les fichiers nécessaires à l'accomplissement de la tâche (fonctionnalité, correction de bogues, refonte). Non seulement cela facilitera le travail du réviseur, mais il sera également facile de repérer et de revenir en arrière en cas de problème majeur en production. Si le code touche plusieurs aspects, suggérez à l'auteur de le diviser en deux ou plusieurs révisions.
5. Comprendre l'objectif de la modification. La description de la PR doit décrire le changement ou renvoyer à un document externe avec des détails (par exemple un ticket Jira ou Trello). Une fois que vous avez compris l'objectif, vérifiez si la modification correspond aux exigences.
6. Si la PR corrige un bogue, le correctif devrait également introduire un ensemble de tests pour traiter ce scénario spécifique et éviter d'autres ruptures à l'avenir.

When (Quand)

Quand devriez-vous consacrer du temps à la lecture du code ?

Je consacre généralement environ 30 minutes, deux fois par semaine, à des PR d'autres équipes, qui ne sont donc pas liés au travail de mon équipe. Cela peut être moins s'il y a une date limite stricte pour mon équipe, ou cela peut être plus si c'est un jour plus calme et que les commentaires ont généré beaucoup de débats.

Lorsque j'ai commencé mon parcours dans le monde de l'iOS, j'avais l'habitude de passer du temps dans les revues de code autant que dans l'étude de Swift et de l'iOS. Cela m'a permis de me familiariser rapidement avec la nouvelle base de code, les idiomes du langage, les meilleures pratiques et de connaître les personnes concernées par le projet.

Je commence généralement à réviser le code dès que je dois apprendre une nouvelle base de code importante.

How (Comment)

Générer des centaines ou des milliers de commentaires peut devenir ennuyeux et répétitif d'une manière ou d'une autre, et vous pouvez avoir tendance à être trop direct. La communication écrite étant différente de la communication orale, je vous suggère de suivre les conseils ci-dessous :

Soyez aimable
Cela se passe d'explications. En général, je commence une critique adressée à quelqu'un que je ne connais pas encore par un simple « Salut 👋 » dans le premier commentaire. Si vous remarquez qu'il manque quelque chose, ne l'exprimez pas par une déclaration telle que « C'est incomplet », mais demandez plutôt : « Y a-t-il une raison pour laquelle il manque quelque chose ? »

Attention aux détails
Essayez de trouver un équilibre entre les changements importants et ceux qui le sont moins (l'expérience vous guidera). Nous ne voulons pas bloquer une fonctionnalité importante ou un bogue de production à cause d'un problème d'espacement !

Demandez l'avis de l'auteur
Si le changement n'est pas évident, ou si vous demandez un refactoring, il est bon de terminer vos réflexions par « Qu'en pensez-vous ? » ou « Quelle est votre opinion à ce sujet ? ».

Les compromis sont parfois inévitables
Certains projets sont plus critiques que d'autres. Essayez de ne pas trop insister sur les petits changements qui risqueraient de retarder la publication. Vous pouvez convenir avec l'auteur de créer une branche de « polissage » plus tard. Cependant, comme je le dis souvent : « plus tard == jamais »

Ne faites pas de suppositions
Vous êtes en train d'inspecter une PR, et vous repérez une erreur macroscopique. De plus, le développeur qui a écrit le code a un titre « junior ». Vous pourriez supposer que cette erreur est due à l'inexpérience et à la méconnaissance de certaines bases. Le code a peut-être été écrit à la hâte, ou le commit a eu lieu de nuit après une dure journée [2]. Ou ... il y a de fortes chances qu'il y ait une motivation cachée et logique derrière tout cela. En cas de doute, dites-le à l'auteur : « Peut-être que quelque chose m'échappe, mais... »

Garder les préférences personnelles facultatives
Lorsque vous suggérez des changements lors des revues de code, assurez-vous que ces changements constituent de réelles améliorations du code et non des préférences personnelles du réviseur, qui ne sont pas nécessairement les mêmes que les meilleures pratiques de l'entreprise ou de l'industrie. J'utilise la formule suivante lorsque je propose un tel changement : « C'est une question de préférence personnelle, mais si vous l'aimez <changement de code> ».

Complimenter l'auteur
« LGTM » (looks good to me) pourrait être interprété comme “J'ai examiné votre PR rapidement”. Personnellement, je suis d'accord quand je lis cela, cependant, n'hésitez pas à écrire dans la PR à quel point le code a été bien écrit (Si vous pensez sincèrement que c'est vrai !). Voici quelques raisons pour lesquelles un éloge est bien mérité : une nouvelle fonctionnalité fantaisiste a été introduite, ou un refactoring complexe mais bien architecturé a eu lieu.

Si la pull request contient un simple changement de couleur ou l'ajout d'un paramètre à une fonction, un compliment extrêmement enthousiaste peut être interprété comme un sarcasme 😅. Calibrez vos mots avec soin.

Effets secondaires

Quelle que soit votre gentillesse ou votre contribution à l'amélioration du code au cours d'une évaluation, il arrive que des personnes se sentent contrariées. Peut-être feront-ils les changements demandés, mais ne vous remercieront pas d'avoir consacré votre temps à leur code.

Quelqu'un rejettera vos suggestions

Peut-être que vos suggestions sont tout simplement erronées, ou inutiles, ou peut-être que l'ego de l'auteur suggère que le fait de modifier le code portera atteinte à son estime de soi. Ce n'est pas grave. Cependant, si vous pensez qu'un bout de code est dangereux ou peut introduire de sérieux problèmes de performance, pensez à impliquer d'autres développeurs dans la discussion, en les étiquetant. Mieux encore, essayez d'en discuter lors d'un appel ou en personne.

Ne vous attendez pas à être récompensé pour vos efforts

Même avec des dizaines de revues de code et d'approbations qui ont aidé les gens à fusionner leur travail, quand c'est votre tour, ne tenez pas pour acquis que vous recevrez de l'attention pour votre nouvelle pull request brillante.

Vous pouvez gagner des alliés

L'année dernière, j'ai effectué des révisions régulières d'une base de code que je ne connaissais pas, dans le but de m'y intégrer rapidement. Il y a eu des discussions et des propositions engageantes avec de nombreux développeurs et, finalement, certains modules ont été modifiés et améliorés. C'était le résultat d'un effort conjoint de l'auteur et de moi-même - le pair évaluateur, un parfait étranger ! J'ai rencontré des gens formidables, qui étaient heureux d'améliorer leur code et restaient toujours ouverts aux suggestions et aux apprentissages. À la fin du trimestre, lorsque je leur ai demandé un retour d'information formel, j'ai reçu des réponses fantastiques. Certains appellent cela du réseautage, je pourrais même utiliser le mot « amitié ».

Conclusion

Dans cet article, nous avons discuté des avantages d'une revue de code cohérente. J'ai fait la promotion de mon approche W3H, qui définit un processus pour donner un feedback sur le code d'une autre personne.

Je l'applique régulièrement dans mon travail quotidien chez Expedia Group™️, où la collaboration est encouragée et formellement définie par notre valeur « Inclure consciemment ».

Pour résumer :

1. Réfléchissez aux avantages qu'il y a à examiner le code de quelqu'un d'autre (réseautage, familiarisation avec une nouvelle base de code, apprentissage de nouveaux idiomes, etc.)
2. Faites-le souvent.
3. Donnez un retour d'information au-delà des limites de votre équipe.
4. Ne faites pas de suppositions.
5. N'attendez pas le même degré de dévouement pour votre pull request.
6. Soyez aimable.

Merci pour la lecture ! Si vous avez des questions ou des commentaires, n'hésitez pas à les laisser ci-dessous. Si vous avez aimé cet article, pensez à applaudir 👏 et à me suivre. Merci !

P.S. : Cet article n'a pas été rédigé à l'aide d'une intelligence artificielle. Je n'ai utilisé que mes capacités cérébrales humaines limitées, mon expérience professionnelle, et j'ai eu du mal à trouver le temps de l'écrire.

Pour en savoir plus

• The Pragmatic Programmer: Your Journey to Mastery, 20ᵗʰ Anniversary Edition, David Thomas, Andy Hunt, Addison-Wesley, 2019
• Agile Technical Practices Distilled, Pedro M. Santos, Marco Consolaro, Alessandro Di Gioia, Packt, 2019

Références

[1] Cette définition et quelques idées utiles sur la façon de produire de meilleures pull requests et d'avoir plus de chances de recevoir rapidement une évaluation positive sont présentées dans cet article très intéressant de Mario Bittencourt.
[2] Si vous avez deviné la citation sans chercher sur le web, écrivez-la dans les commentaires, et vous ferez de moi un dev heureux 😃.

Source : "The Importance of Being a Code Reviewer" (par Carlo Sales)

Et vous ?

Quel est votre avis sur le sujet ?

Voir aussi :

Revue de code : un guide en 5 étapes pour que vos collègues vous détestent, par Mensur Durakovic

7 signes révélateurs d'un code illisible : Comment identifier et résoudre le problème, par Mensur Durakovic

Gagnez du temps sur les révisions de code et la planification de projet avec l'analyse statique, par Kateryna Shlyakhovetska, Group Product Manager chez Jetbrains