Le majeur de Chesterton
(arp242.net)- 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
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
La personne suivante qui lira le code a donc une chance non nulle de comprendre pourquoi c’est comme ça
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
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 fixerUn 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
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 nouveauMa 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
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é
git logoujj log, mais presque toujours via l’affichage des annotations de ligneLa 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
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 »
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
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é