1 points par GN⁺ 3 시간 전 | 1 commentaires | Partager sur WhatsApp
  • La barrière de Chesterton conseille de ne pas modifier à la légère du code dont on ne comprend pas la raison d’être, mais du code et des historiques de commit qui ne laissent pas cette raison reportent la même charge sur les développeurs qui suivent
  • Sur ce dépôt, le corps des commits des 13 dernières années ne représente que 295 lignes au total, et tombe à 167 lignes si l’on retire les messages liés à dependabot, aux revert et aux typos
  • Les titres de commit, même pour de gros changements, n’apportent pas de contexte avec des formulations comme « fix page A », et il n’y a presque ni documentation séparée ni commentaires dans le code, ce qui rend la raison des changements difficile à retracer
  • Le code contient des refactorings inachevés, des restes de fonctionnalités supprimées, des fonctionnalités ajoutées mais ni reliées ni utilisées, et des fonctionnalités que personne ne semble utiliser
  • Les messages de commit et la documentation devraient au minimum laisser une trace de « ce qui change, pourquoi ça change et pourquoi c’est une bonne solution » ; sinon, le coût pour les développeurs qui arrivent ensuite augmente fortement

Le poids laissé par du code sans explication

  • La barrière de Chesterton est une métaphore qui dit qu’il ne faut pas modifier à la légère quelque chose tant qu’on n’a pas compris pourquoi c’est ainsi
    • En programmation aussi, on peut “corriger” du code qui paraît étrange, puis découvrir plus tard qu’il y avait une raison à cette bizarrerie
  • Ce cas met en lumière le problème inverse
    • Il y a beaucoup de code et de décisions étranges, mais aucun historique n’explique pourquoi ils en sont arrivés là
    • Tous les développeurs précédents sont partis, et il n’y a plus personne à qui poser la question
  • L’historique des commits du dépôt ne fournit pratiquement aucun contexte utile
    • Le corps des commits des 13 dernières années totalise 295 lignes
    • Si l’on exclut manuellement les corps de commits dependabot, « revert commit » et « fix typo », on tombe à 167 lignes
    • Cela revient à peu près à une ligne par mois
  • Les titres de commit, le plus souvent du style « fix page A », sont eux aussi insuffisants pour expliquer des changements importants
  • Il n’y a pas de documentation séparée, et presque pas de commentaires dans le code
  • Dans une telle situation, on se rapproche davantage du majeur de Chesterton : « nous avons fait beaucoup de choses étranges, mais nous ne vous dirons pas pourquoi »

Le minimum qu’un développeur doit laisser derrière lui

  • La base de code contient plusieurs types de code inachevé ou résiduel
    • des refactorings non terminés
    • des restes de fonctionnalités supprimées
    • des fonctionnalités ajoutées mais ni reliées ni utilisées
    • des fonctionnalités que personne ne semble utiliser
  • Dans l’ensemble, cela ressemble aussi fortement à un problème de fossé de Chesterton
    • On est proche d’une situation du type « il n’y a pas encore de clôture, alors construisons-en une », sans se demander si une clôture est réellement nécessaire
  • Bien écrire est difficile, mais laisser une explication simplement correcte n’est pas difficile
  • Un historique de changements devrait fondamentalement répondre à trois questions
    • Qu’est-ce qui change ?
    • Pourquoi cela change-t-il ?
    • Pourquoi cette solution est-elle bonne ?
  • Il arrive que « Implement new feature X » suffise, mais dans la plupart des cas il y a quelque chose à dire sur le pourquoi ou le comment de l’ajout d’une fonctionnalité
  • Pour un correctif de bug, un refactoring ou un changement substantiel, on peut généralement laisser au moins un ou deux paragraphes expliquant le changement et sa raison d’être
  • Ce type de trace n’est pas optionnel : cela fait partie du travail du développeur logiciel
    • Il n’est pas nécessaire d’écrire avec élégance
    • Il n’est pas nécessaire d’écrire un anglais parfait
    • Il n’est pas nécessaire de rédiger un grand essai
    • Même si certains éléments manquent, c’est toujours bien mieux que de ne rien laisser du tout
  • Ne rien laisser, c’est reporter le problème sur tous ceux qui viendront ensuite

1 commentaires

 
GN⁺ 3 시간 전
Avis sur Lobste.rs
  • Il arrive qu’on ne sache pas clairement, sur le moment, ce qui deviendra important plus tard. Quand tout le processus ayant mené jusqu’au commit est consigné publiquement, c’est d’une grande aide, mais comme toutes les personnes concernées ont déjà beaucoup de contexte, certains éléments passent à la trappe parce qu’ils semblent « trop évidents »
    Si les discussions ne sont pas archivées, il devient bien plus difficile de reconstituer le processus de décision comme dans une archéologie numérique. Au final, on doit parfois gérer une situation sans savoir pourquoi cette clôture existe, et il faut alors l’évaluer à partir du contexte actuel et de la connaissance du système qu’ont les personnes présentes aujourd’hui
    Avoir quelqu’un qui est resté longtemps sur le projet et a acquis une compréhension globale du système ainsi qu’une intuition est d’une grande aide. Les organisations qui considèrent les développeurs comme des rouages interchangeables font que personne ne reste assez longtemps, répètent les mêmes erreurs et réinventent sans cesse la roue

    • Le plus grand bénéfice du code review, c’est qu’une autre personne lit le code et pousse à ajouter des commentaires partout où la raison n’est pas évidente. J’ajoute déjà des commentaires là où ce n’est pas clair pour moi, donc désormais, s’il reste un commentaire, c’est qu’au moins l’une des deux personnes a jugé qu’une explication était nécessaire
      La personne suivante qui lira le code a donc une chance non nulle de comprendre pourquoi c’est comme ça
    • C’est similaire aux anciennes recettes de cuisine qui n’indiquaient pas des ingrédients que « tout le monde savait où trouver », ou aux musiques médiévales dont on supposait que « tout le monde connaissait » le rythme et la mesure. Aujourd’hui, on ne sait parfois plus ce qu’il y avait dedans
  • Je n’ai jamais compris les développeurs qui mettent simplement « fix » ou « WIP commit » dans leurs messages de commit. Ils n’ont sans doute jamais vraiment fait d’archéologie du code, ou n’ont même jamais envisagé qu’on puisse avoir à le faire
    J’essaie toujours de me tromper dans le sens d’un surplus d’information. Ainsi, mon moi futur, mon successeur ou la pauvre personne d’astreinte quand un incident éclate auront au moins une chance de comprendre pourquoi quelque chose a fini par casser

    • C’est peut-être le signe que la taille de l’unité de travail n’est pas la bonne. Pour certains développeurs, un commit peut n’être qu’une étape parmi des centaines de petites étapes nécessaires pour produire une unique fonctionnalité « vendable »
      Dans ce cas, il est difficile de suivre mentalement ce qui a changé après chaque point de contrôle ou d’y associer une explication pertinente, et le commit est alors traité moins comme un moyen de découper le travail en unités bien définies que comme le bouton « sauvegarder » d’un jeu vidéo
      À l’inverse, si un gros commit résulte d’un vaste refactoring d’API, tout message qui n’équivaut pas à une note de conception a de fortes chances d’être insuffisant, et si on ne va pas faire cet effort, l’intérêt d’un long message devient lui aussi discutable. Cela dit, certaines personnes prennent cela comme un insigne d’honneur et finissent par écrire dans les notes de version des formules du type « corrections de bugs et améliorations de fonctionnalités », ce qui est clairement une mauvaise conclusion
  • Beaucoup de développeurs oublient souvent que « l’autre personne » qui devra lire et comprendre le contexte d’un changement, ce peut être eux-mêmes dans le futur. Tout le monde connaît ce moment où l’on se gratte la tête devant un bloc de code, lance git blame, et voit son propre nom en train de vous fixer
    Un bon message de commit m’a d’innombrables fois évité des heures, voire parfois des jours, passés à fouiller dans des tickets, des e-mails ou des logs de chat pour retrouver le pourquoi. Même dans des cas où, en théorie, j’aurais dû pouvoir répondre immédiatement
    Il faut être bienveillant envers son soi futur. Mieux vaut déverser dans l’historique git tout ce qu’on savait, pensait et discutait. On ne sait jamais clairement ce qui sera encore évident dans cinq ans

    • Bien sûr, une partie de ce savoir a peut-être davantage sa place dans des commentaires ou dans des documents de conception que dans l’historique des commits
    • Le côté déstabilisant de voir mon propre nom dans git blame, je le vis au moins rarement quand il s’agit de quelque chose qui avait une raison d’être. Quand il s’agit de comportements aléatoires ressemblant à des bizarreries venues d’ailleurs, il est difficile de fournir une explication utile sans pouvoir observer le processus de l’autre côté, mais si c’était quelque chose qui avait du sens pour moi à un moment donné, en le relisant, j’arrive en général à le comprendre de nouveau
      Ma manière d’apprendre ressemble sans doute davantage à des couches de lave qui s’accumulent. Depuis le lycée, elle n’a pas tellement changé en profondeur ; j’ai surtout continué à empiler ce que je sais et les manières de l’utiliser
  • Récemment, quelqu’un a affirmé que si un message de commit dépasse une phrase, c’est généralement une perte de temps, et j’ai eu envie de m’y opposer fermement, mais j’ai été moins habile que prévu pour démontrer le contraire
    Une question est de savoir quelles informations doivent aller dans le message de commit, et lesquelles doivent aller dans des commentaires inline, des ADR, ou d’autres documents de forme plus longue
    J’essaie toujours d’écrire de bons messages de commit, mais au travail c’est déjà sans espoir, et même sur mes projets perso expérimentaux je n’arrive pas à être cohérent

    • Pour moi, les messages de commit sont faits pour les reviewers. Ils indiquent pourquoi ce changement est nécessaire, ce qu’il corrige ou pourquoi on veut une nouvelle fonctionnalité, et ils fournissent un cadre pour comprendre la modification
      Mais le code final doit rester compréhensible sans avoir à lire le message de commit. S’il y a dans le nouveau code des éléments qui demandent une justification, cela doit aller dans les commentaires
      Autrement dit, le message de commit explique pourquoi on a fait ce changement maintenant, tandis que les commentaires expliquent pourquoi le code, une fois la modification terminée, est dans cet état
      Pour des changements plus importants, surtout de nouvelles fonctionnalités, il devrait y avoir un document de conception quelque part. S’il doit être relu, cela peut être un vrai document dans le dépôt, ou bien se trouver dans le gestionnaire d’issues
      Le message de commit devrait pointer vers ce document, et il peut aussi devoir expliquer les détails apparus lorsque la conception a été traduite en code. Si possible, il est bien d’inclure aussi un court résumé du document de conception. Quand plusieurs personnes peuvent relire, c’est particulièrement important pour indiquer qui devrait être le plus concerné, et pour repérer les cas où quelqu’un qui aurait dû donner un avis à l’étape de conception a été oublié
    • Mon critère pour décider quoi mettre dans un message de commit, c’est que je ne les consulte généralement pas via git log ou jj log, mais presque toujours via l’affichage des annotations de ligne
      La ligne de titre est universellement très utile pour décider s’il faut creuser davantage. Quand le corps contient des informations sur la raison du changement, c’est utile lorsque cette raison n’est pas intuitive
      Par exemple, même si pas mal de code a été ajouté, « admin: add impersonation » suffit souvent à lui seul. Mais pour « auth: shorten JWT timeouts », j’aimerais voir une ou deux phrases expliquant pourquoi il a fallu réduire les délais d’expiration
      J’ai tendance à penser que les messages de commit vraiment longs sont en pratique assez peu utiles. C’est globalement pour les mêmes raisons que celles soulignées dans l’article. Ce format semble venir de workflows où le message de commit fait aussi office de description de PR, comme les workflows basés sur l’e-mail ou Gerrit. Dans ce cas ce n’est pas nuisible, mais je ne suis pas convaincu que cela apporte nécessairement de la valeur
    • J’ai déjà écrit un article pour répondre précisément à cette question. J’y présentais les différents emplacements de documentation sous forme de hiérarchie et j’y résumais des règles empiriques pour décider où mettre l’information
      Je suis dans une situation similaire. Dans le groupe élargi au travail, nous ne sommes que deux, moi compris, à écrire des messages de commit détaillés
  • Même si l’on sait pourquoi une clôture a été construite, on peut ne pas savoir pourquoi elle est encore là aujourd’hui. Même si l’on est la personne même qui a érigé la clôture de Chesterton, on ne sait pas forcément si on peut la démolir
    L’arbre des interdépendances voulu au moment où le système a été conçu n’est qu’un sous-ensemble de l’arbre réel des interdépendances à un moment donné. Donc, même si un développeur parfait vous explique entièrement pourquoi la clôture a été construite, l’utilité de cette information reste limitée
    Ce qu’ils savent, c’est pourquoi elle a été construite, pas la réponse à « qu’est-ce qui casse si on enlève cette clôture ? ». Il suffit de voir https://xkcd.com/1172/. Parmi les cas d’usage amusants, certains peuvent sembler sans importance, mais il existe toujours des usages légitimes que même le développeur d’origine ne pouvait pas connaître
    Savoir ce que le développeur d’origine pensait, ou ce qu’il avait fumé, c’est bien, mais cette information se situe quelque part entre l’incomplétude et l’absence de pertinence
    Exemple inventé : imaginons que la clôture de Chesterton ait été installée à l’origine, à l’époque où il y avait des fermes des deux côtés, pour empêcher les enfants de s’approcher d’une mare. Aujourd’hui, une autoroute a été construite, et cette clôture est peut-être devenue par hasard le seul dispositif empêchant des collisions entre des cerfs et des voitures, avec à la clé des morts massives d’animaux et d’humains
    Personne ne le sait. Non seulement la combinaison « autoroute + absence de clôture » n’a jamais été testée, mais elle n’a même jamais existé, et les constructeurs de l’autoroute comme le ministère des ressources naturelles ignorent pourquoi il y a si peu de collisions avec la faune sur cette route. Dans quelques années, quand toutes les fermes auront été remplacées par des lotissements et que ce ne sera plus un couloir majeur de déplacement animal, la clôture deviendra peut-être inutile, ou peut-être pas
    Si cela vous paraît tiré par les cheveux ou sans rapport avec votre situation, je vous envie. D’après mon expérience, à part dans les entreprises d’une certaine taille et pas trop anciennes, c’est globalement comme ça que les choses se passent
    La vérité, c’est que tout ce que nous faisons fait partie d’un écosystème, dépend de choses avec lesquelles nous n’avons jamais explicitement convenu d’interagir, et sert en retour de dépendance à d’autres. On peut réduire la surface de l’API et éviter que tous les détails d’implémentation deviennent l’affaire du voisin, mais les couplages involontaires relèvent presque d’une loi de l’univers aussi inévitable que l’augmentation de l’entropie
    Pour certaines personnes, cela sonne comme du nihilisme ou du défaitisme. On pourrait dire qu’il faut lutter contre l’entropie. Mais je pense que le meilleur usage du temps et le meilleur retour sur investissement commencent par reconnaître qu’il ne s’agit pas fondamentalement d’un ennemi à combattre, mais de quelque chose à gérer
    Faire comme si l’on pouvait toujours connaître l’état du monde, c’est se garantir l’échec et l’auto-culpabilisation. Un peu comme le 100 % de disponibilité : cela n’existe pas, et c’est un mauvais objectif pour la plupart des sujets
    Si l’on admet que l’on gère un processus avec un certain degré d’incertitude de type heisenbergien, on peut choisir comment utiliser efficacement le temps limité dont on dispose chaque jour pour produire de meilleurs résultats. En particulier, cela permet un arbitrage intelligent entre réponses préventives et réponses réactives, et de comprendre qu’on ne peut pas réduire la réaction à zéro, et qu’il n’a parfois aucun sens de faire un an de prévention pour éviter une seule journée de réaction
    Alors, combien de documentation faut-il écrire dans un commit ? Combien faut-il de documents de conception ou de plans de test ? Je n’en sais rien. S’il faut proposer une piste, c’est que toute documentation est écrite pour ses lecteurs
    Si vous modifiez une base de code, les membres actuels de l’équipe comme les nouveaux arrivants doivent pouvoir, par investigation, comprendre ce que le changement a fait et pourquoi il a été fait, avec aussi quelques avertissements sur les appuis fragiles ou les bugs qui supportent de la charge
    Cela prend en général la forme non pas d’une prose interminable, mais de pointeurs vers du contexte supplémentaire qui plante le décor. Par exemple : « L’authentification a été exigée à cette étape dans le cadre de la politique imposant une approbation multipartite pour tout changement. see: go/multiparty »

    • Le fait que l’entropie augmente et que des couplages involontaires continuent d’apparaître peut même être vu non comme un défaut, mais comme une propriété utile empêchant les systèmes de se figer dans un état permanent
      Les systèmes qui recherchent la perfection tout en devant composer avec des humains sont vraiment pénibles. DRM, trusted computing, remote attestation, Faro Plague, smart contracts, ce genre de choses
      Un système qu’on peut redémarrer en mode service pour le corriger est bien meilleur. Parce qu’on ne peut pas prédire dans quelle direction le logiciel devra évoluer pour être réellement utile aux gens à l’avenir. Mieux vaut le rendre facile à modifier que de le verrouiller à 100 %
  • Nous n’écrivons presque jamais de corps de commit, mais nous écrivons plutôt bien les titres. Si c’est la métrique, je ne sais pas très bien ce que cette métrique mesure
    Dans une grande base de code, du code clair et une couverture de tests suffisante sont souvent bien plus utiles que la documentation

    • Pas d’accord. Il arrive que le changement réel et sa raison soient tous deux subtils. Quand on essaie de comprendre pourquoi du code bizarre est comme il est, regarder les tests modifiés dans ce commit est une étape supplémentaire, et cela ne dit pas forcément pourquoi ils ont changé
      Il arrive qu’un changement parfaitement valable soit fait et que les tests soient mis à jour en conséquence, mais qu’avec le recul il soit totalement flou de comprendre pourquoi ce changement était nécessaire. C’est encore plus vrai si les lignes modifiées provoquent en production des comportements inattendus ou supplémentaires
      Un simple revert n’est pas forcément souhaitable, et dans ce cas, avoir l’historique complet des raisons du changement aide vraiment
      J’ai souvent vu des cas où l’idée était bonne, mais où des conséquences inattendues apparaissaient. Connaître l’intention permet alors d’aboutir au changement réellement correct, qui corrige le nouveau problème tout en préservant la raison initiale de la modification
      Si vous tenez à des commits d’une seule ligne, mettez au moins un numéro de ticket pour qu’on puisse y lire l’historique
  • J’ai gagné pas mal d’argent pendant cinq ans à voyager dans des régions exotiques pour remettre d’aplomb ce genre de bases de code. @arp242, il faut toujours augmenter ses tarifs et dormir avec https://archive.org/details/working-effectively-with-legacy-code sous l’oreiller

  • Heureusement, les générateurs de bric-à-brac IA nous écrivent des messages de commit gigantesques. Comme ils ont souvent un certain rapport avec le changement réel, on peut considérer qu’au moins ce problème-là est réglé

    • Vraiment ? Pour résumer ce qui a changé, ça peut être correct, mais j’ai du mal à croire qu’ils soient bons pour expliquer pourquoi cela a changé