LCC 258 - Les mineurs de fond

18 views
Skip to first unread message

Emmanuel Bernard

unread,
Jun 16, 2021, 4:52:45 AMJun 16
to lescast...@googlegroups.com
Arnaud, Antonio et Emmanuel discutent des actu du développeur en cette période pré estivale.
Du Spring Boot, du Hibernate, du Vert.x, du web qui tombe, du Gradle 7, des mineurs et des méthodologies autour des pull requests.


Emmanuel

Nicolas Delsaux

unread,
Jun 28, 2021, 2:53:40 AMJun 28
to lescast...@googlegroups.com

On n'est pas vendredi mais ...

En écoutant cet épisode, je suis frappé par votre discussion sur les PR.

J'ai développé peu à peu la conviction que, malgré l'intérêt qu'elles peuvent avoir pour permettre à l'équipe de partager la connaissance du code, les PR sont en fait globalement toxiques.

Typiquement, dans ce qu'Arnaud dit sur le fait que ce soit du travail non planifié, on sent bien que la PR est une charge pour celui qui va la revoir. Parce qu'en fait, revoir une PR implique de se plonger dans le problème au même niveau que la personne qui a écrit le code revu, sans pour autant présenter l'intérêt que pourrait avoir le fait d'écrire soi-même le code. Par ailleurs, on se met en position de critique (au sens critique artistique), qui est une position loin d'être facile.

Enfin, relire une PR monte difficilement à l'échelle : relire 5 modifications dans 3 fichiers, ok. Relire 200 modifications dans 80 fichiers, c'est juste impossible. Alors, comment faire ?

A mon avis, il vaudrait mieux remplacer la revue de code par la conception écrite par un développeur qui écrit aussi les tests et l'écriture de tests aussi offensifs que les défauts du code peuvent le permettre

(j'ai fait ça il y a déja presque 20 ans, et ça marchait du feu de dieu)

La revue de conception permet de partager la compréhension de la solution en cours de développement, et l'écriture de tests "offensifs" permet de sortir du happy path et d'avoir une solution qui marche dans la plupart des cas.

Enfin, le fait de définir des rôles de développeurs va rendre le flux de travail plus visible.

Parce que si les PR traînent, c'est peut-être aussi qu'elles n'apparaissent pas suffisamment dans les outils de suivi de production (le kanban/scrumban/whatever)

--
Vous recevez ce message, car vous êtes abonné au groupe Google Groupes "lescastcodeurs".
Pour vous désabonner de ce groupe et ne plus recevoir d'e-mails le concernant, envoyez un e-mail à l'adresse lescastcodeur...@googlegroups.com.
Cette discussion peut être lue sur le Web à l'adresse https://groups.google.com/d/msgid/lescastcodeurs/CAEW2RjKY4TnkvA_Q5Jmze5u2%2BZDnW-81Q9oWxMNvuk58y-yW7Q%40mail.gmail.com.

Mickael Istria

unread,
Jun 28, 2021, 3:53:28 AMJun 28
to lescast...@googlegroups.com
Salut,

Typiquement, dans ce qu'Arnaud dit sur le fait que ce soit du travail non planifié, on sent bien que la PR est une charge pour celui qui va la revoir. Parce qu'en fait, revoir une PR implique de se plonger dans le problème au même niveau que la personne qui a écrit le code revu, sans pour autant présenter l'intérêt que pourrait avoir le fait d'écrire soi-même le code. Par ailleurs, on se met en position de critique (au sens critique artistique), qui est une position loin d'être facile.

Oui, la revue de code de contributions non planifiees est une charge, un tres grosse charge meme. Mais c'est un mal necessaire pour permettre a des gens contributeurs de gagner en expertise et pour le reviewer de gagner en confiance dans ces gens pour qu'in fine, la confiance et l'experise soient suffisamment hauts pour permettre aux nouveaux contributeurs de faire des revues a leur tour, de faire des patches pour lesquels tu les laisses merger sans review, et petit a petit prendre une part de responsabilite, de liberer des "anciens" qui pourront travailler sur des choses qui ont plus de valeur pour eux.

Enfin, relire une PR monte difficilement à l'échelle : relire 5 modifications dans 3 fichiers, ok. Relire 200 modifications dans 80 fichiers, c'est juste impossible.

Une revue de 200 modifications dans 80 fichiers, ca se fait si tu en as besoin. Si en tant que reviewer tu vois ca et cette modification ne te fait pas envie ou ne t'inspire pas confiance (trop compliquee ou quoi que ce soit), tu demandes a l'auteur de la separer en plus petits morceaux. Si il dit non, tant pis pour lui, tant qu'il ne te paye pas, tu lui dois rien a priori (mais il faut pas le snober car c'est peut-etre ce contributeur qui s'occupera de faire le plus de revues dans 6 mois si tu le mentores bien, et c'est peut-etre grace a lui que dans 6 mois tu auras suffisamment de temps decharge pour implementer la fonctionalite de tes reves que les revues t'empechent de faire en ce moment).

A mon avis, il vaudrait mieux remplacer la revue de code par la conception écrite par un développeur qui écrit aussi les tests et l'écriture de tests aussi offensifs que les défauts du code peuvent le permettre

Ca c'est dans une equipe fermee et dediee. Dans un projet ouvert avec une equipe elastique/fluctuante et hors de tout management commun, tu ne peux pas demander a un autre contributeur de faire une conception ecrite a priori s'il en a pas envie, il ne le fera pas sauf s'il est convaincu de l'interet pour lui-meme, pour le projet, du ROI, et que faire cette doc est plus rentable que de 1. faire autre chose ou 2. ecrire le code lui meme, et aussi qu'il a suffisamment confiance en toi pour croire -sans engagement contractuel de ta part- que tu feras ta part du boulot et donc que tu creeras la valeur attendue apres que lui ait investi dedans. Bref, ca demande au reviewer trop de travail et confiance trop tot pour que ca soit rentable en general.
Avec la revue, tu es quelques etapes plus loin dans l'engagement: le contributeur a deja fait beaucoup de travail sans rien te demander, c'est une forme de politesse que de prendre connaissance de son travail. En tant que committer/admin sur un project communautaire, une contribution de code doit etre recue avec honneur et respect.

La revue de conception permet de partager la compréhension de la solution en cours de développement, et l'écriture de tests "offensifs" permet de sortir du happy path et d'avoir une solution qui marche dans la plupart des cas.

Ceci n'est pas lie aux PR a mon avis. Tu peux ecrire des tests offensifs dans une PR et d'autres peuvent faire des revues; beaucoup de projets sur lesquels je bosse font ca, plutot en mode TDD: lorsque quelqu'un rapporte un bug, avant meme d'avoir la solution, on peut ecrire des tests qui reproduisent le bug (ou specifient le comportement attendu), mettre ca dans une revue, voir le build qui se casse les dents sur les tests et il ne restera plus qu'a les rendre verts. Les reviewers peuvent regarder les tests, les solutions proposees.
Bref, je pense que la revue est un outil de collaboration, et ce dont tu parles (tests offensifs et cie) est plus une methode de conception. Les deux ne sont pas exclusifs du tout.

En fait, si tu es dans un projet OSS communautaire. Quelle alternative vois-tu a un contributeur qui envoie du code (en PR, MR, patchset, patch... peu importe la forme) a un reviewer qui decide de quoi en faire? Une aternative, c'est l'equipe et la confiance totale: pas de revue; mais dans l'OSS communautaire ca scale encore moins, surtout dans les projets ou l'expertise est assez concentree sur peu de gens; tu as besoin de recruter de nouveaux contributeurs, la revue est un levier pour ca.

Je te renvoie aussi au livre "Economics of Software Quality" qui aborde ces sujets et les differents impacts sur la qualite; cependant, il occulte beaucoup la partie "productivite" qui peut parfois etre l'ennemi de la "qualite". C'est peut-etre ca le nerf de la guerre, comment concilier qualite et productivite dans differents types de projets. Mon impression c'est qu'en OSS communautaire, la balance entre les 2 se trouve dans la revue de code.

Bertrand Delacretaz

unread,
Jun 28, 2021, 4:53:22 AMJun 28
to lescast...@googlegroups.com
On Mon, Jun 28, 2021 at 8:53 AM Nicolas Delsaux <nicolas...@gmx.fr> wrote:
> ...A mon avis, il vaudrait mieux remplacer la revue de code par la conception écrite par un développeur
> qui écrit aussi les tests et l'écriture de tests aussi offensifs que les défauts du code peuvent
> le permettre..

J'aime bien l'idée.

Certains projets utilisent un mécanisme de propositions ("proposals")
qui sont censées être discutées et acceptées avant de créer un nouveau
module ou une contribution majeure.

Par exemple https://github.com/apache/cordova-discuss - je pense que
c'est une bonne manière de poser un cadre pour les contributions
importantes, tout en acceptant des pulls requests "sorties de nulle
part" pour des choses plus simples.

-Bertrand
Reply all
Reply to author
Forward
0 new messages