• # Mainteneurs grincheux

    Posté par (site web personnel, Mastodon) . En réponse au journal LLVM se fait de vieux os ? La recherche pour rester jeune.. Évalué à 10.

    Je sais que c'est dans l'air du temps de discuter des "mainteneurs grincheux", alors je vais prendre un peu le contre-pieds, en étant régulièrement justement du côté "mainteneur" (ou "reviewer" de code, ce qui peut souvent être vu similairement pas un contributeur) de la barrière.

    La partie du message à laquelle — j'imagine — tu fais référence en parlant de mainteneur grincheux qui introduit de la friction dans le développement, est:

    L'autre inconvénient c'est que souvent ces personnes se retrouvent à dire "non, on ne va pas faire comme ça, il faut plutôt faire comme ça", et ça ne plaît à personne quand la personne en face essaie juste de faire intégrer un changement qu'on lui a demandé de faire.
    <original>The other downside is that it often involves saying "no, let's not do that, let's do this instead" a lot, and that makes people feel bad when they know someone else is just trying to achieve a task someone assigned them.</original>

    Moi quand j'ai lu cette phrase, je n'ai pas du tout interprété ça comme toi, mais plutôt qu'y a beaucoup de contributeurs qui ne veulent pas se faire chier, ni même forcément faire du bon code, et surtout pas interagir avec l'équipe et collaborer. Donc de leur point de vue, tout se passe bien si on leur dit "c'est génial, on intègre", mais dès qu'y a un peu de revue détaillée et qu'on leur demande de changer leur code pour intégration, y a plus personne au bout de la ligne (ou du mécontentement).

    Daniel Berlin parle des gens à qui on a assigné une tâche (donc typiquement des gens qui contribuent essentiellement parce qu'ils sont payés pour le faire; notons que ça ne signifie pas toujours qu'ils aiment l'idée du Libre, ou de l'OpenSource, plus que ça). Mais il y a d'autres cas. On voit souvent des gens qui font vraiment du lâchage de patch, mais dès qu'on répond avec une revue de code (pas méchante, polie et amicale, mais on leur demande de changer des trucs), on n'a pas de réponse. Bien sûr, ça peut aussi être des problèmes de manque de temps (ça m'arrive moi-même régulièrement de laisser passer des mois avant de revenir sur un patch car j'avais d'autres priorités). Des fois aussi, les jeunes, étudiants ou développeurs moins expérimentés ont beaucoup de mal à corriger leur patch en suivant les indications donnés, etc. Y a plein de raisons possibles.

    Pourquoi leur demande-t-on de corriger leur patch? Certains (beaucoup même, j'ai l'impression) projets ont cette tendance à intégrer tout et n'importe quoi. Parfois ils corrigeront après coup, dans un nouveau commit, parfois même pas, alors que le patch de base a clairement des manques, ne suit pas le style du projet, etc. Je pense que l'envie d'avoir de nouveaux contributeurs pour certains mainteneurs surpasse l'envie d'avoir du code de haute qualité. Surtout pour les plus petits projets.
    Dans GIMP, on a tendance à demander un haut niveau de qualité pour un patch. Déjà qualité du code même, mais aussi le respect de l'architecture du logiciel (on ne peut pas inclure des headers de n'importe quelle partie du code vers n'importe quel autre partie. Par exemple le core de GIMP n'a aucune notion de GUI, etc.), ou même le style de code. C'est la base: si je contribue sur un projet tiers, je regarde toujours quel est leur style (tabulations, espaces, accolades en début ou fin de ligne?...). On s'en fiche quel style j'aime personnellement quand je contribue. J'impose mon style seulement sur mes projets persos.
    Alors bien sûr, c'est un choix et les 2 se défendent: vaut-il mieux tout intégrer et faire plaisir aux contributeurs (quitte à prendre beaucoup de temps après coup pour tout nettoyer derrière) ou demander un haut niveau d'entrée et risquer de perdre des contributeurs (en se faisant appeler "grincheux")? Ce que j'ai tendance à penser sur le sujet:

    • beaucoup de petits patchs seraient bien plus vite et bien mieux écrits si on les faisaient nous-même que si on en fait la revue et le commentaire. Alors pourquoi le fait-on? Parce qu'avoir des contributeurs est important et qu'on "utilise" du temps maintenant mais pour potentiellement en gagner beaucoup plus tard si certains contributeurs décident de rester dans le coin, et pourquoi pas devenir des contributeurs majeurs. C'est donc un investissement de temps sur le long terme.
      Alors d'un certain point de vue, vaut-il mieux faire plaisir à un max de ces petits contributeur au cas où on a notre futur contributeur majeur dans le lot? D'un autre côté, si on ne l'a pas habitué à faire de la qualité dès le début, à quoi s'attend-t-on pour la suite? Veut-on vraiment de quelqu'un qui n'est pas capable de suivre un style donné, de revenir sur son code, d'écouter des critiques constructives, de lire et comprendre des revues de code et réorganiser le sien? Peut-être que cela est bien plus rentable sur le long terme. Après tout, comme je le disais, intégrer les petits patchs peut vraiment prendre un temps fou, donc autant que ce temps soit utilisé à bon escient pour le futur du programme. Parfois même, le patch en soi est moins important que la relation qui peut se construire avec le contributeur.

    • on essaie d'avoir un code toujours stable sur master, même s'il est expérimental. Il ne faut pas intégrer un commit qui ne compile pas, ou va faire crasher le programme, ou autre mauvais "comportement". Intégrer un patch qu'on sait ne pas être parfait est donc une mauvaise idée. Évidemment on peut rétorquer que si on fait immédiatement un commit de correction derrière le patch du contributeur et on pushe les 2 en même temps, alors on a un changement atomique (quelqu'un qui fetche le dépôt aura les 2 commits d'un coup), mais cela reste non-idéal.
      Une autre possibilité peut être de corriger direct le commit du contributeur (--amend). On le fait souvent d'ailleurs, surtout quand les seuls changements sont des problèmes de style de code. Parfois aussi pour des changements plus importants, voire parfois on fait du gros élagage. Mais on laisse le nom du contributeur en tant qu'auteur du commit. Y a la question alors: jusqu'à quel niveau de changement peut-on toujours considérer ce contributeur comme auteur du commit? C'est pour ça que je rechigne à faire trop souvent cela personnellement, hormis pour des changements vraiment mineurs, et fait plutôt des commits juste après. Mais idéalement on préférerait que les contributeurs nous fassent un patch parfait qu'on n'aura pas à corriger. Et pour cela, ils doivent simplement suivre nos indications de revue de code.

    Ensuite il y a les choix de design de l'application. On ne peut pas accepter tout et n'importe quoi. Et beaucoup de contributeurs semblent penser que changer un "petit" (d'après eux) truc n'a pas besoin d'être discuté. Déjà beaucoup de choses ne sont pas si "petites" même si un contributeur va essayer de vous en convaincre (or s'il est si attaché à voir ce patch intégré, c'est justement que le changement n'est pas si petit à ses yeux). Ensuite chaque fonctionnalité a son fan-club. Même si on se rend compte qu'une fonctionnalité est fondamentalement mauvaise, il y aura toujours quelqu'un pour l'apprécier. Et ça veut dire qu'une fois acceptée, elle sera très dure à retirer plus tard sans se prendre des plaintes et des menaces. En gros quoiqu'on fasse (garder, retirer, changer...), y a toujours quelqu'un pour ne pas être content et le faire savoir, souvent de façon virulente.
    XKCD: Workflow

    XKCD 1172 par Randall Munroe — CC BY-NC.

    Ça signifie donc qu'il vaut bien mieux bien réfléchir sur chaque changement de fonctionnalité et chaque nouvel ajout, avant de l'accepter. Pas se dire "on pourra toujours revenir sur la décision plus tard". Car ce n'est pas similaire. Dans un cas, il ne se passe rien; dans l'autre on se prend des coups quoiqu'on fasse. Il est donc tout naturel de calmer les ardeurs et de réfléchir posément les nouvelles fonctionnalités, non seulement sur leur qualité intrinsèque, mais aussi sur le choix d'implémentation. Et donc oui, des fois, ça en revient à dire "non on va pas faire ça, on va faire ça à la place" (ce qui n'est pas un refus, mais une alternative ou différente implémentation — souvent pour des raisons qualitatives du code, donc maintenabilité — pour un comportement final similaire). Et oui, des fois, ça plaît pas forcément.

    Mais ça veut pas forcément dire qu'on est grincheux. :P
    Perso j'essaie d'être toujours le plus agréable et ouvert possible aux nouvelles idées. Mais ça veut pas dire que je dois laisser mon esprit critique au vestiaire et me passer de prendre des décisions finales quand il le faut. C'est pas facile tous les jours, d'ailleurs. Ensuite, soyons clair: globalement cela se passe bien et la plupart des contributeurs le prennent bien. Mais il arrive des couacs par moment.

    En tous cas c'est assez marrant de voir que selon les personnes et leur niveau d'implication dans des projets, à quel point on a un niveau de lecture différent de la même phrase. ;-)

    Film d'animation libre en CC by-sa/Art Libre, fait avec GIMP et autre logiciels libres: ZeMarmot [ http://film.zemarmot.net ]