La revue de code exige de lire
(hauleth.dev)- La revue de code n’est pas une formalité avant déploiement, mais un processus qui transfère la responsabilité des incidents, des problèmes de sécurité et des suppressions de données accidentelles d’une seule personne vers une responsabilité d’équipe
- De meilleures evals, de meilleurs tests, les feature flags, les garde-fous et l’observabilité peuvent être des réponses visant à réduire l’anxiété liée au déploiement de code non lu, mais cela est critiqué comme une approche qui passe à côté de l’objectif de partage de responsabilité de la revue
- Exiger une approbation sans lecture revient à demander à quelqu’un d’appuyer sur un bouton sans réfléchir, ce qui mène à la satire de la button roulette, où l’on fusionne au hasard une PR dont toute la CI est verte
- La revue de code fait voir à différents membres de l’équipe plusieurs parties d’une grande base de code, ce qui réduit le bus factor et sert de mécanisme d’apprentissage pour que les nouveaux membres s’approprient le code et la culture du code
- Pour imposer le déploiement en production de code non lu, il faudrait une décharge écrite de responsabilité sur les bugs, les problèmes de sécurité, les pannes, etc., et la conclusion est qu’il est difficile d’obtenir une telle décharge
Le but de la revue est le partage de responsabilité, pas l’approbation
- Le point de départ est la question « de quoi a-t-on besoin pour être à l’aise à l’idée de déployer en production du code qu’on n’a pas lu ? », tirée du texte de Charity Majors « AI enthusiasts are in a race against time, AI skeptics are in a race against entropy »
- Les réponses comme de meilleures evals, des tests, des feature flags, des garde-fous, de l’observabilité, l’isolation des dépendances, la réduction du rayon d’explosion, ou le fait de commencer par de petits changements hors des chemins critiques sont présentées comme des réponses qui évitent le cœur du sujet
- Le but de la revue est de ne pas laisser à une seule personne la charge des incidents, des problèmes de sécurité ou des suppressions accidentelles de données, mais de la répartir entre l’auteur et l’ensemble des relecteurs comme responsabilité d’équipe
- Si l’on exige « approuver sans lire », la raison de faire cliquer manuellement un humain devient faible, d’où la satire appelée
button roulette, qui consiste à fusionner aléatoirement l’une des PR assignées dont toute la CI est verte
Ce qu’une revue sans lecture fait perdre
- La revue de code oblige les membres de l’équipe à regarder d’autres parties de la base de code, ce qui compense la limite selon laquelle, dans un système trop vaste, personne ne peut en connaître en permanence toutes les parties
- Le processus de revue réduit le bus factor et aide plusieurs membres de l’équipe à mieux se familiariser avec la base de code et la culture du code
- Si tout le monde approuve sans lire, on perd cet effet d’apprentissage, et non seulement le bus factor remonte à 1, mais le problème est aussi externalisé à un tiers
- La conclusion est que, pour accepter une demande de déployer en production du code non lu, la personne qui donne cette instruction devrait fournir une décharge écrite de responsabilité couvrant les bugs, les problèmes de sécurité, les temps d’arrêt, etc.
1 commentaires
Commentaires sur Lobste.rs
Je rejette fortement l’idée que l’objectif d’une review soit de diluer la responsabilité
Si du code fusionné dans main casse la production, c’est la responsabilité de son auteur, pas la mienne ni celle de toute l’équipe
En tant que professionnel, on signe son travail ; si la conception d’un pont tamponnée par une licence P.E. en génie civil tue des gens, cela reste aussi son travail et sa responsabilité
La responsabilité de l’équipe, en tant que développeurs et gardiens de la codebase, est de faire en sorte que personne ne puisse casser la production
Une fusion dans main signifie que quelqu’un a relu, examiné les changements, discuté de la conception, demandé plusieurs révisions puis approuvé
Personne ne construit seul la tour Eiffel, et blâmer un individu au travail ne produit que du ressentiment et une culture toxique
Si c’est un schéma de comportement répété, c’est à traiter par les RH
Si la responsabilité est nulle, le reviewer ne sert à rien et il n’y a aucune raison de faire une review
La dilution de responsabilité est davantage une conséquence ; l’objectif principal est de détecter des erreurs qu’il est facile de laisser passer seul mais plus difficile d’ignorer à plusieurs
Cela dit, je pense qu’on abuse des reviews dans le logiciel, et qu’une review ne peut pas être un substitut à l’ingénierie
Je ne pense pas qu’assimiler « approuver sans lire le code » à « approuver sans réfléchir » soit exact
Par exemple, que se passerait-il si le programme était accompagné d’une preuve de correction, si l’on pouvait lire les définitions et les théorèmes dans la PR, si la CI vérifiait avec un outil déterministe que le programme et la preuve correspondent, et si les exigences non fonctionnelles comme le contraste, les benchmarks de performance ou la latence de queue étaient aussi contrôlées, tandis que les changements d’UI pouvaient être facilement manipulés dans un prototype ?
Même dans un tel monde, on peut se demander s’il serait nécessaire d’être aussi obsédé qu’aujourd’hui par la lecture ligne par ligne du code
Encore aujourd’hui, la plupart des gens qui écrivent du SQL ne vérifient pas une par une que le plan d’exécution correspond exactement à ce qu’ils veulent
Le billet original me semble plus proche de « définissons un monde idéal où l’on peut se sentir à l’aise pour déployer sans littéralement lire le code », puis il demande « comment passer en douceur du présent à ce monde ? »
On peut débattre de la distance qui sépare l’industrie, ou une entreprise et une codebase données, de cet objectif, mais si l’on imagine sérieusement ce monde idéal, la plupart des gens pourront citer quelques critères
Je comprends qu’avec la dynamique actuelle de l’industrie on l’interprète comme « sans réfléchir », mais je ne pense pas que ce soit ce que Charity veut dire
Et cela exige de lire le code, aussi bons que soient les tests
Si Alice ajoute du code et que Bob le review, puis qu’Alice part en vacances, Bob doit connaître suffisamment cette partie et s’en sentir suffisamment responsable pour pouvoir la corriger ou y ajouter une nouvelle fonctionnalité
Si Bob se contente d’apposer son tampon sur la preuve de correction, il ne sera pas prêt ensuite à travailler sur cette codebase
Même s’il a participé au processus de commit, si ses connaissances et son sentiment d’appropriation n’ont pas augmenté, cela revient pratiquement au même que de ne pas avoir participé
Sauf qu’un compilateur est déterministe, alors qu’un LLM est un outil non déterministe qui génère des tests potentiellement douteux
Nous avons déjà des programmes qui transforment des instructions ou du code écrits par des humains en code lisible par machine ou en représentations intermédiaires, et comme ils garantissent une certaine relation entre l’entrée et le résultat généré, on peut faire confiance à la sortie sans devoir nécessairement l’inspecter
Il arrive qu’on lise la sortie d’un compilateur avec un outil comme Godbolt pour des raisons d’optimisation, mais sinon ce n’est généralement pas nécessaire
Essayer de faire cela à un autre niveau d’abstraction non déterministe peut sembler plausible, mais ce n’est qu’une imitation des garanties de correction fournies par le compilateur, et cela finira par poser problème
À un niveau plus fondamental, je pense que le débat sur les LLM est le symptôme d’un problème : les langages de programmation déterministes existants ne sont pas suffisamment expressifs et leurs outils ne sont pas suffisamment efficaces
Les LLM peuvent donner l’impression de résoudre ce problème, mais ce n’est pas la vraie solution ; c’est plutôt ajouter une couche d’indirection supplémentaire et un coût en performance sur une base déjà trop limitée
Cela ressemble davantage à un pseudo-compilateur coûteux et bogué, fonctionnant au-dessus d’un interpréteur, lui-même au-dessus de code machine recompilé
C’est pourquoi je pense que c’est une impasse technique
Pour les entreprises, cela peut être une voie vers des profits à court terme, mais je ne crois pas que cela améliorera le logiciel ni la relation que les humains ont avec le fait de l’utiliser et de le créer
Le logiciel est plus qu’une simple technique pour livrer un produit
Il est utile de distinguer les cas d’usage
S’il s’agit simplement de pousser un nouveau JavaScript pour l’UI d’une application interne, je pense qu’il est acceptable de ne pas forcément examiner le CSS aussi