La revue de code peut être meilleure

• Les développeurs sont nombreux à être insatisfaits de l’expérience de revue de code sur GitHub, et de nouvelles tentatives sont en cours pour l’améliorer
• Un outil expérimental appelé git-review est conçu pour gérer la revue de code directement en local avec le code, plutôt que via une interface web dans le navigateur
• La revue est gérée sous la forme d’un seul commit ; les commentaires de revue sont laissés comme des annotations dans le code, et le relecteur comme l’auteur modifient ce commit à tour de rôle
• Cependant, si le code est modifié ou rebasé en cours de revue, cela crée des difficultés, notamment pour la gestion des conflits et l’usage de --force-with-lease, ce qui a empêché l’approche de rencontrer un grand succès
• En fin de compte, le retour à la revue via le web s’est imposé, mais l’idée d’inclure directement l’état de la revue dans le dépôt Git reste séduisante, et de meilleures alternatives pourraient émerger à l’avenir avec l’amélioration de Git, notamment via l’adoption d’un Change-Id à la Gerrit

---

Prise de conscience des problèmes des systèmes de revue de code

• Aujourd’hui, beaucoup de personnes sont mécontentes du processus de revue de code de GitHub
• Les principaux problèmes sont le manque de prise en charge des pull requests empilées et de la revue interdiff, ainsi que :
  • l’état de la revue n’est pas stocké dans le dépôt
  • la revue via une interface web orientée distant est obligatoire
• Le problème, à mes yeux, tient au manque de décentralisation de la revue et à l’inefficacité de l’interface

Comparaison des workflows d’écriture de code et de revue

• Lorsqu’ils écrivent du code, les gens utilisent un éditeur en local
  • c’est un environnement avec peu de latence mémoire et NVMe, optimisé pour les workflows particuliers de chacun
• Pour la revue de code aussi, il est préférable de pull la branche source en local pour travailler
  • avec des outils comme Magit, il est possible d’explorer non seulement le diff mais aussi tout le contexte du code
  • on peut profiter d’un environnement de développement puissant : exécuter les tests, aller à la définition du code, essayer des refactorings, etc.
• En revanche, pour laisser un retour sur une PR, il faut passer par le navigateur et une interface web lente, avec même une forte latence de saisie sur les gros diffs

Interface de revue de code idéale et structure de stockage

• En pratique, il est plus naturel de laisser des commentaires directement en ligne dans le code ou de modifier le code soi-même

  // CR(matklad): Hm, this check seems imprecise to me.  
  // Shouldn't we compare `replica.view` instead of `header.view` here?  
  if (header.view != view) return;  
  

• Lorsque les données sont stockées dans une base distante plutôt que dans le dépôt git local, cela entraîne aussi des problèmes de latence et de vendor lock-in

L’idée de git-review et le retour d’expérience réel

• L’idée de git-review est la suivante :
  • la revue de code prend la forme d’un commit unique au sommet de la branche de PR
  • des commentaires de code avec marqueurs spéciaux sont ajoutés dans ce commit
  • le relecteur et l’auteur modifient ce commit chacun leur tour, dans une collaboration fondée sur push --force-with-lease
  • quand tous les commentaires sont marqués comme résolus (//? resolved), un commit de revert est ajouté à la fin de la revue pour conserver une trace
• L’idée est simple et pratique, mais en réalité les problèmes suivants apparaissent :
  • lorsqu’on modifie le code pendant la revue, les conflits avec les commentaires sont fréquents dans les commits inférieurs ou les nouveaux commits
  • le processus de force-push augmente les frictions de collaboration et la complexité du travail
  • il devient difficile de gérer les désalignements entre l’historique des modifications du code et l’avancement de la revue, ainsi que les conflits de fusion

Nouveaux changements et possibilités futures

• À l’avenir, un Change-Id à la Gerrit dans Git upstream pourrait être introduit
  • cela faciliterait le suivi de l’historique des modifications par commit et étendrait probablement la prise en charge de la revue interdiff
  • mais cela pourrait aussi entrer en conflit sur certains points avec l’approche git-review
  • avec cette nouvelle structure de Change-Id, des approches originales comme l’ajout de commentaires de revue directement dans le commit pourraient devenir possibles

Conclusion et présentation de systèmes utiles à connaître

• Pour l’instant, la situation est donc revenue à une revue de code basée sur une interface web
• Le besoin d’une meilleure solution demeure
• Présentation de systèmes et d’outils connexes à connaître
  • Fossil : un système SCM qui conserve toutes les informations dans le dépôt
  • NoteDb : intègre dans git l’historique de stockage de l’état de revue de Gerrit
  • git-bug : stocke les informations d’issues dans git
  • git-appraise : conserve les informations de revue dans git lui-même
  • prr : implémente une interface de revue dans l’éditeur via l’API GitHub
  • How Jane Street Does Code Review : présente un exemple d’une réalité meilleure
  • git-pr : un projet qui remplace l’ensemble du workflow de PR par des fonctionnalités natives de git

Conclusion

• Il n’existe pas encore de solution parfaite, et les tentatives pour offrir une meilleure expérience développeur se poursuivent
• Les attentes sont fortes quant aux évolutions à venir

[roxie]

Je ne sais pas si l’approche de git-review est bonne, mais je suis d’accord sur le fait que les revues de PR basées sur GitHub sont épouvantables..

[GN⁺]

Avis Hacker News

• Ce qui m’a longtemps frustré dans la code review, c’est que les retours vraiment utiles — au-delà des remarques de pure préférence — arrivent presque toujours trop tard. Le seul résultat possible, ou presque, finit par être : « il faut tout reprendre avec une nouvelle conception » ou « il n’aurait jamais fallu faire ce travail au départ ». On dirait que la code review est le seul moment où toutes les parties prenantes participent réellement et réfléchissent sérieusement au changement. Il y a peut-être eu des discussions en réunion ou dans des tickets Jira, mais souvent, c’est seulement quand quelqu’un d’une autre équipe ou d’un autre service reçoit la notification de code review qu’il découvre le changement. Moi aussi, quand une autre équipe implémente quelque chose de bizarre, c’est souvent la notification de review qui me l’apprend. Il est irréaliste d’attendre que tout le monde suive tout à l’avance. À l’université dans les années 90, on faisait aussi des design reviews, mais je n’en ai jamais vu dans le monde professionnel. Et je ne pense pas non plus qu’une design review garantisse de détecter tous les problèmes à l’avance.

  • Dans le monde du génie logiciel, il y a finalement très peu de véritable ingénierie. L’industrie accepte mal la lenteur inhérente à un vrai processus d’ingénierie. La plupart des logiciels ne sont pas critiques au point d’être vitaux, et les bugs ou erreurs peuvent être corrigés plus tard. Les enjeux et la possibilité de patcher n’ont rien à voir avec des domaines comme les ponts, les usines ou les moteurs d’avion, où l’échec n’est pas permis.

  • Notre équipe est un petit groupe de 4 à 6 développeurs. Quand on a une première ébauche mentale d’une nouvelle fonctionnalité, on en parle tout de suite avec les collègues. Comme tout le monde fait pareil, les code reviews se concentrent surtout sur des détails comme les code smells, et l’architecture globale est généralement décidée à l’avance par 2 ou 3 personnes. Si les membres de l’équipe ne sont pas d’accord avec le code, ils hésitent à toucher au code des autres, et la situation se dégrade. Je pense que ça peut continuer à bien fonctionner à plus grande échelle si le partage des responsabilités est bien fait.

  • Si toutes les parties prenantes ne participent qu’au moment de la code review, ce n’est pas un problème de Git ou du système de gestion de versions, c’est un problème d’organisation. Cela signifie qu’on ne partage pas le contexte de création de la PR, les discussions dans les tickets ni le processus de décision. C’est le signe d’une organisation dysfonctionnelle ; c’est comme critiquer le processus éditorial sous prétexte que tout le monde ne s’implique sérieusement qu’au moment où le livre imprimé sort.

  • Dans notre organisation, on rédige obligatoirement une RFC pour les décisions de conception fondamentales. Ce qui constitue une décision de conception fondamentale est évalué en continu, de manière autonome, au sein de l’équipe. Si, au stade epic dans Jira, la manière détaillée d’implémenter n’est pas encore définie, on commence par assigner la rédaction d’une RFC. Une RFC peut être réservée à notre équipe ou viser toute l’équipe logicielle ; dans ce second cas, tout le monde peut la lire et laisser des commentaires avant la réunion qui a lieu toutes les deux semaines. Même si c’est contraignant, c’est bien mieux qu’un environnement sans processus de conception collaborative basé sur des RFC.

  • D’après mon expérience, je comprends bien les remarques sur les design reviews. Avant, on faisait des documents de conception formels et des revues, puis on est passés au prototypage et à la conception itérative. On omettait souvent des détails importants au stade de la conception, et comme on y avait déjà investi beaucoup de temps, on avait ensuite tendance à laisser passer les choses. Réunir toute l’équipe pour relire était aussi inefficace, donc les problèmes étaient finalement découverts pendant la code review. Beaucoup de gens écrivent mal les documents de conception ou manquent de motivation pour le faire. Au final, dès qu’on dépasse 5 personnes, ce genre d’inefficacité semble inévitable. L’environnement idéal serait un PO, les principaux utilisateurs et environ cinq développeurs.

• Je trouve très intéressant de voir des discussions sur les stacked pull requests sur HN. Quand graphite.dev a démarré, à l’époque, ceux qui n’avaient pas d’expérience chez Facebook ou Google connaissaient rarement ce type de workflow. C’est fascinant de voir à quel point les tendances en matière de code review changent vite en trois ou quatre ans.

  • En tant qu’ancien utilisateur d’arcanist à l’époque pré-Mercurial, je fais encore activement la promotion de Graphite auprès des équipes qui souffrent de grosses PR, de merge commits, etc. Je suis particulièrement impressionné par l’audace et le résultat de leur intégration avec les PR. J’aimerais que Graphite propose un mode prescriptive qui initialise le repo avec une configuration plus hardcore, afin de pouvoir faire des hypothèses plus fortes.

  • Graphite est une excellente solution, mais son prix reste assez élevé, ce qui complique la persuasion des décideurs. J’aimerais voir un outil aussi bon que Graphite devenir open source ou être intégré directement dans GitHub.

  • Le workflow fig me manque parfois.

  • Avec le récent incident de sécurité chez CodeRabbit, je suis réticent à tester de nouveaux outils qui intègrent des LLM avec la base de code. Une expérimentation intéressante peut vite se transformer en problème de sécurité.

  • Les stacked pull requests ajoutent fondamentalement une complexité dont on pourrait se passer. Des changements fréquents et de petite taille constituent une bien meilleure pratique de développement. Le trunk-based development ou la continuous integration correspondent beaucoup mieux à cet objectif.

• De plus en plus de développeurs semblent s’accorder sur ce à quoi un outil de review devrait ressembler. À ce stade, ce qui compte, c’est l’organisation et les acteurs capables d’en faire quelque chose de réellement applicable et durable. L’adoption récente de git change-id est une bonne avancée (merci à jj, git butler, gerrit, etc.). Graphite et GitHub se concentrent sur des solutions réservées à leurs propres utilisateurs, pas sur une approche ouverte à tous. Et les innombrables outils en ligne de commande côté client n’ont pas non plus beaucoup d’impact. Les éléments vraiment nécessaires sont :

  • pouvoir fonctionner entièrement en local
  • avoir un support officiel de l’équipe core pour vscode (avec une bonne UX spécifique à l’intégration vscode)
  • réutiliser au maximum les assets UI avec les interfaces web comme vscode web et avec d’autres éditeurs
  • fournir les fonctions de base via une CLI ou une bibliothèque, avec des frontières claires pour l’extension
  • prendre en charge commit/branch/stacked commit/agent snapshot/dev self-review
  • intégrer nativement les signaux CI/CD, en s’appuyant sur l’excellent travail d’UI montré par Meta
  • être très fine-grained selon le contexte et modifiable à chaque étape (par exemple, dans Cursor on peut accepter ligne par ligne ; la code review humaine devrait aussi permettre ce niveau de granularité)
  • être totalement incremental, avec un moyen simple de ne review que les modifications lors des mises à jour de PR, au lieu de tout revoir

• Mon plus gros reproche envers GitHub, c’est la lenteur extrême de l’application. À un point où l’onglet du navigateur finit parfois par se figer. Azure DevOps est de loin le meilleur outil de code review que j’aie utilisé.

  • J’ai utilisé Azure DevOps pour faire du développement .NET dans un environnement Microsoft, et c’est vraiment un outil qui s’intègre bien à l’écosystème .NET.

  • Je me demande si tu as déjà beaucoup utilisé GitLab. Parmi les big 4, c’est GitLab que je préfère.

  • Je suis curieux de savoir ce qui t’a autant plu dans DevOps. Je l’utilise tous les jours, mais je le trouve assez similaire à GitHub. Au contraire, il m’arrive même de regretter la fonctionnalité de GitHub qui applique automatiquement les modifications suggérées.

  • La combinaison d’un usage massif de JavaScript et de la pression pour livrer vite produit ce genre d’environnement lent. Cela dit, c’est quand même mieux qu’Atlassian.

• L’idée d’utiliser git directement pour la code review est séduisante. C’est pratique de pouvoir manipuler les changements localement. Je ne comprends pas pourquoi il faudrait absolument limiter la review à un seul commit — une approche où le reviewer committe directement ses commentaires ou corrections sur la branche de la PR me paraît aussi intéressante. C’est une sorte d’hybride entre le github flow traditionnel et le flux Linux basé sur mailing lists et patchs.

  • Je me demande si les PR GitHub sont vraiment en lecture seule. J’ai déjà vu un membre d’équipe proposer une correction via un commentaire « suggestion », puis l’appliquer à ce commit en un clic.

  • L’idée qu’un commit de review doive être unique est étrange. Si plusieurs personnes reviewent, cela crée aussi des problèmes d’édition concurrente, et ça ne correspond pas bien au code. Il est plus naturel que chacun travaille séparément sur sa propre branche de review, puis qu’on finisse par squash & rebase. Si la discussion s’allonge, on pourrait même garder des comment commits chaînés. L’important, c’est que ces données existent quelque part en dehors de la branche main.

• Quand je fais une code review, je préfère tirer les changements dans une branche locale, faire un soft-reset, puis les regarder comme si c’était moi qui les avais écrits. Si les commits sont mal découpés, chaque collaborateur doit les redécouper de son côté pour review, ce qui est inefficace. Et si l’objet de la review est trop vaste, il est difficile de dire que quelqu’un le comprend entièrement. Le tout n’est pas simplement la somme des parties.

  • Je partage un exemple de fonction shell pour faciliter la code review. À partir d’un arbre propre, elle checkout automatiquement une branche de review prédéfinie, ouvre le diff dans nvim, puis nettoie la branche une fois le travail terminé.

  • Bien savoir construire de bons commits et les PR qui en découlent est une compétence tout aussi importante que savoir bien coder. Pourtant, en pratique, il y a plus de développeurs qu’on ne le pense qui sont mauvais pour découper une PR ou écrire de bons messages de commit.

  • Dans une équipe de data science qui travaille en parallèle, on utilise deux ou trois branches checkoutées pour la code review.

  • Pour reviewer des PR, j’utilise https://github.com/sindrets/diffview.nvim dans Neovim. L’interface ressemble au diff de vscode tout en exploitant le mode diff de vim. Pour les reviews légères, git log -p --function-context est aussi utile.

• Je m’intéresse à une façon de travailler où une personne écrit le premier draft, puis une autre l’affine et le merge. Je me demande si certains l’ont déjà pratiquée.

  • Je suis d’accord avec un style où plus de 90 % du codage réel est fait par une seule personne, et où le reviewer prend directement la responsabilité de l’intégration finale et du merge. Dans mon précédent poste, c’était toujours le reviewer qui se chargeait du merge, et pour les gros changements il se contentait de transmettre des commentaires. C’est bien plus efficace que de devoir demander à quelqu’un de cliquer sur un bouton d’approbation à chaque fois.

  • Quand on participe à l’écriture du code reviewé, on a l’impression de pouvoir donner un feedback beaucoup plus profond. Ce mode de fonctionnement aide à voir le code comme « notre code » plutôt que « mon code / ton code ». Cela convient bien à une culture itérative et collaborative, comme avec la TDD.

  • Cette approche ressemble à de la programmation en binôme asynchrone.

  • Je connais des gens qui pratiquent le pair programming dans un cadre de trunk-based development. Ils codent à deux, puis si c’est OK, ils mergent directement dans main et déploient aussitôt après le passage des tests. En pratique, ça fonctionne bien.

• J’utilise l’extension GitHub Pull Request dans VSCode pour reviewer localement, et c’est plutôt pratique. On peut commenter et reviewer directement depuis l’éditeur.

  • Sur le web GitHub, quand il y a beaucoup de fichiers modifiés, certains fichiers sont masqués ; dans VSCode, en revanche, on peut naviguer librement, ce qui rend l’expérience bien plus confortable. Puisque cette fonctionnalité existe dans la combinaison VSCode/GitHub, j’imagine qu’elle pourrait aussi s’étendre à d’autres éditeurs.

  • C’est vrai que c’est mieux, mais le vendor lock-in entre GitHub et VSCode reste très fort dans les deux sens.

  • Ouvrir une PR sur GitHub web puis appuyer sur la touche “.” pour reviewer directement dans VSCode web est aussi une bien meilleure expérience.

• Je suis ravi d’entendre que Git va intégrer des change ID de première classe ! Cela ressemble au suivi des révisions de diffs dans Phabricator chez Facebook. Je serais curieux d’avoir des liens pour en savoir plus.

  • Le problème fondamental, c’est que git ne gère pas correctement l’information de branche. Fossil se souvient de la branche sur laquelle un commit a été créé. Une task branch joue en soi le rôle de change ID. Bien sûr, comme git doit aussi permettre de réécrire l’historique, la solution n’est pas simple.

• J’aimerais aussi mentionner Sourcehut. Il reprend le flux classique de patchs, tickets, bugs et discussions par e-mail, avec intégration aux mailing lists et à la CI. Drew Devault fournit aussi des ressources sur l’envoi et la réception de patchs via git-send-email.io et git-am.io.