L’objectif théorique de la revue de code est de permettre à un développeur de s’améliorer grâce à la critique constructive de ses collègues.
Alors déjà ça, c'est prendre de haut les gens dont on relit le code. Forcément avec une attitude pareille, ça risque de pas marcher.
Pour moi, en particulier (mais pas seulement) dans un projet open source, l'objectif de la revue de code, c'est plutôt, en tant que relecteur, de s'assurer qu'on comprend bien le code qu'on va merger et devoir ensuite maintenir.
Dans mes projets pro, c'est fréquent que les nouveaux arrivants commencent par faire de la revue de code dans leurs premières tâches. Ça leur permet de poser plein de questions sur l'architecture du projet et donc de comprendre comment ça marche. Beaucoup plus efficacement qu'en écrivant du code puis en se faisant "démolir" (c'est le mot utilisé dans l'article) quand il est relu.
Je te rejoins complètement, et ce n'est pas le seul truc qui me chiffonne dans cet article.
D’expérience, je pense qu’il faut remplacer la revue de code par la revue de conception.
C’est-à-dire demander d’abord aux développeurs de définir comment ils vont résoudre un problème en écrivant le plan de leur solution technique. Et ensuite de relire et de commenter cette conception.
Ça c'est n'importe quoi aussi. J'ai vu un paquet de fois des belles planifs sur tableau blanc tomber à l'eau dès la première ligne de code. Croire qu'on peut résoudre toutes les questions d'implémentation sans coder, et que si ça ne marche pas c'est qu'il fallait réfléchir mieux, c'est se fourvoyer. C'est même la raison pour laquelle on ne fait plus de cycle en V.
Pour moi la question de la conception doit se faire « en gros », et le moment du chiffrage est un bon moment pour échanger là dessus : on communique sur les approches et on élimine les plus gros problèmes.
Les daily/le stand-up sert à remonter les soucis qu'on rencontre sur le terrain : on communique sur les problèmes, on les élimine ou on les contourne.
La revue sert à partager l'information et affiner des détails : on communique sur comment les choses sont faites en pratique. C'est autant au bénéfice du relecteur que du codeur.
Dans toutes les boîtes où je suis passé ces trois étapes étaient la raison pour laquelle les projets étaient fluides. Trois points de synchro pour communiquer et le reste du temps on est bien tranquille. Les seuls soucis que j'ai eu étaient des galères d'humains de mauvaise volonté qui se pointaient au stand-up en râlant à propos du commercial (c'est pas le sujet) ou du patron (toujours pas le sujet), ou encore des gens qui ne voulaient pas retoucher à leur code crado en revue parce que « boaf, ça marche comme ça », voire des héros fiers bloqués en refusant d'échanger à propos de leurs galères. Au final ça me semble simple : si tu joues en équipe, ça marche bien, sinon c'est la merde.
Je vous rejoins totalement et j'ajoute un point :)
Parce que ces deux solutions au problème de cohérence arrivent en fait au mauvais moment. En effet, elles arrivent après que le code a été écrit.
Le problème c'est de faire une revue trop tard. Sauf cas particulier (ou contexte qui ne sont pas ce que je connais), il faut fusionner son code au plus tôt. Outre les effets tunnels (dont la pratique de la revue permet dans une certaines mesure de l'éviter), c'est l'objet de l'intégration continue. Intégrer fréquemment son code ce n'est pas avoir un outil qui lance régulièrement un build et des tests, mais bien le faire de fusionner fréquemment son code. Cela permet d'avoir des revues bien plus courtes (et donc de ne pas overflow le relecteur) et de ne pas risquer de remettre en cause des semaines de travail.
Je sais que ça peut choquer/surprendre de ne pas attendre qu'une fonctionnalité soit complète pour la fusionner, mais avoir des incréments les plus petits possibles aident vraiment (je trouve) pour un tas de raisons.
Une autre solution que je n'ai pas expérimenté serait de créer la merge request très tôt et de demander une revue régulière, mais on perd l'intégration fréquente.
La revue sert à partager l'information et affiner des détails
Et pas à corriger les bugs ! Contrairement à ce que certains pensent.
Je sais que ça peut choquer/surprendre de ne pas attendre qu'une fonctionnalité soit complète pour la fusionner, mais avoir des incréments les plus petits possibles aident vraiment (je trouve) pour un tas de raisons.
Je suis d'accord avec ce que tu dis. Mais cela peut impliquer un changement culturel. Car, il y a un moment où tu auras du mal à n'avoir qu'une partie de fonctionnalité sans feature flag. Et ça ne passe pas dans la culture de certaines équipes/entreprises.
l'objectif de la revue de code, c'est plutôt, en tant que relecteur, de s'assurer qu'on comprend bien le code qu'on va merger et devoir ensuite maintenir.
tu pratiques une forme de binômage …la programmation extrême…
“It is seldom that liberty of any kind is lost all at once.” ― David Hume
# L'objectif de la revue de code
Posté par pulkomandy (site web personnel, Mastodon) . Évalué à 10.
Alors déjà ça, c'est prendre de haut les gens dont on relit le code. Forcément avec une attitude pareille, ça risque de pas marcher.
Pour moi, en particulier (mais pas seulement) dans un projet open source, l'objectif de la revue de code, c'est plutôt, en tant que relecteur, de s'assurer qu'on comprend bien le code qu'on va merger et devoir ensuite maintenir.
Dans mes projets pro, c'est fréquent que les nouveaux arrivants commencent par faire de la revue de code dans leurs premières tâches. Ça leur permet de poser plein de questions sur l'architecture du projet et donc de comprendre comment ça marche. Beaucoup plus efficacement qu'en écrivant du code puis en se faisant "démolir" (c'est le mot utilisé dans l'article) quand il est relu.
[^] # Re: L'objectif de la revue de code
Posté par Julien Jorge (site web personnel) . Évalué à 8.
Je te rejoins complètement, et ce n'est pas le seul truc qui me chiffonne dans cet article.
Ça c'est n'importe quoi aussi. J'ai vu un paquet de fois des belles planifs sur tableau blanc tomber à l'eau dès la première ligne de code. Croire qu'on peut résoudre toutes les questions d'implémentation sans coder, et que si ça ne marche pas c'est qu'il fallait réfléchir mieux, c'est se fourvoyer. C'est même la raison pour laquelle on ne fait plus de cycle en V.
Pour moi la question de la conception doit se faire « en gros », et le moment du chiffrage est un bon moment pour échanger là dessus : on communique sur les approches et on élimine les plus gros problèmes.
Les daily/le stand-up sert à remonter les soucis qu'on rencontre sur le terrain : on communique sur les problèmes, on les élimine ou on les contourne.
La revue sert à partager l'information et affiner des détails : on communique sur comment les choses sont faites en pratique. C'est autant au bénéfice du relecteur que du codeur.
Dans toutes les boîtes où je suis passé ces trois étapes étaient la raison pour laquelle les projets étaient fluides. Trois points de synchro pour communiquer et le reste du temps on est bien tranquille. Les seuls soucis que j'ai eu étaient des galères d'humains de mauvaise volonté qui se pointaient au stand-up en râlant à propos du commercial (c'est pas le sujet) ou du patron (toujours pas le sujet), ou encore des gens qui ne voulaient pas retoucher à leur code crado en revue parce que « boaf, ça marche comme ça », voire des héros fiers bloqués en refusant d'échanger à propos de leurs galères. Au final ça me semble simple : si tu joues en équipe, ça marche bien, sinon c'est la merde.
[^] # Re: L'objectif de la revue de code
Posté par barmic 🦦 . Évalué à 3.
Je vous rejoins totalement et j'ajoute un point :)
Le problème c'est de faire une revue trop tard. Sauf cas particulier (ou contexte qui ne sont pas ce que je connais), il faut fusionner son code au plus tôt. Outre les effets tunnels (dont la pratique de la revue permet dans une certaines mesure de l'éviter), c'est l'objet de l'intégration continue. Intégrer fréquemment son code ce n'est pas avoir un outil qui lance régulièrement un build et des tests, mais bien le faire de fusionner fréquemment son code. Cela permet d'avoir des revues bien plus courtes (et donc de ne pas overflow le relecteur) et de ne pas risquer de remettre en cause des semaines de travail.
Je sais que ça peut choquer/surprendre de ne pas attendre qu'une fonctionnalité soit complète pour la fusionner, mais avoir des incréments les plus petits possibles aident vraiment (je trouve) pour un tas de raisons.
Une autre solution que je n'ai pas expérimenté serait de créer la merge request très tôt et de demander une revue régulière, mais on perd l'intégration fréquente.
Et pas à corriger les bugs ! Contrairement à ce que certains pensent.
https://linuxfr.org/users/barmic/journaux/y-en-a-marre-de-ce-gros-troll
[^] # Re: L'objectif de la revue de code
Posté par bbo . Évalué à 1.
Je suis d'accord avec ce que tu dis. Mais cela peut impliquer un changement culturel. Car, il y a un moment où tu auras du mal à n'avoir qu'une partie de fonctionnalité sans feature flag. Et ça ne passe pas dans la culture de certaines équipes/entreprises.
[^] # Re: L'objectif de la revue de code
Posté par Lutin . Évalué à 3.
On ne lit pas de la manière une PR du lead dev et celle du stagiaire qui est arrivé il y a 1 mois.
[^] # Re: L'objectif de la revue de code
Posté par Gil Cot ✔ (site web personnel, Mastodon) . Évalué à 1.
tu pratiques une forme de binômage …la programmation extrême…
“It is seldom that liberty of any kind is lost all at once.” ― David Hume
# C'est pas Xkcd, mais...
Posté par Tonton Th (Mastodon) . Évalué à 2.
https://www.commitstrip.com/fr/2021/04/09/the-secret-of-a-successful-code-review
[^] # Re: C'est pas Xkcd, mais...
Posté par Gil Cot ✔ (site web personnel, Mastodon) . Évalué à 2. Dernière modification le 14 avril 2021 à 17:02.
Tu m'as devancé
=D“It is seldom that liberty of any kind is lost all at once.” ― David Hume
Suivre le flux des commentaires
Note : les commentaires appartiennent à celles et ceux qui les ont postés. Nous n’en sommes pas responsables.