khalil Drissi Cms

Étiquette : Collaboration

  • Bonnes pratiques de revue de code

    Bonnes pratiques de revue de code

    La revue de code est l’habitude au plus fort levier qu’une equipe puisse avoir, et c’est aussi celle qu’on rate le plus souvent. J’ai recu des revues qui ressemblaient a un interrogatoire et des revues qui approuvaient 800 lignes avec un pouce leve en quatre secondes. Les deux sont des echecs. Une bonne revue attrape de vrais problemes, diffuse la connaissance et laisse l’auteur soutenu plutot que juge.

    Relisez ce que les humains font bien

    Ne gaspillez pas votre attention sur le formatage, l’ordre des imports ou la longueur d’une ligne. Un linter et un formateur automatique s’en chargent, et ils ne se fatiguent jamais. Si votre equipe se dispute sur le style dans les commentaires de PR, vous avez un manque d’outillage, pas un probleme de discipline. Configurez le formateur, validez la config, et passez a autre chose.

    Ce qu’une machine ne peut pas verifier, c’est si le code est correct, s’il resout le vrai probleme, et si quelqu’un dans six mois le comprendra. C’est la que va mon attention :

    • Est-ce que ca fait ce que la description annonce ?
    • Que se passe-t-il aux bords : entree vide, valeurs nulles, acces concurrent, un appel reseau qui pend ?
    • Y a-t-il une approche plus simple cachee derriere celle-ci ?
    • Le nommage aura-t-il du sens pour quelqu’un qui n’etait pas dans la piece ?

    Gardez des changements assez petits pour etre vraiment relus

    La limite dure de la qualite de relecture, c’est la taille. La recherche et mon experience disent la meme chose : au-dela de quelques centaines de lignes, la detection de defauts s’effondre parce que les relecteurs survolent. Quand je recois une PR enorme, je demande a l’auteur de la decouper. Une serie de petites PR ciblees recoit un vrai examen sur chacune. C’est aussi pourquoi je rebase et ecrase avec soin avant d’ouvrir une PR, une habitude que j’ai decrite dans les bonnes pratiques Git.

    Commentez comme un collegue, pas comme un compilateur

    Le ton fait l’essentiel du travail. Le meme point passe completement differemment selon la formulation. Je pose des questions au lieu de rendre des verdicts, et je clarifie quels commentaires sont bloquants et lesquels sont optionnels.

    # Au lieu de :
C'est faux.

# Essayez :
Que se passe-t-il ici si items est vide ? Je crois
qu'on depasserait la fin. On pourrait proteger avec
une verification de longueur, ou ce cas est-il
impossible en amont ?

# Et etiquetez les details :
detail : on pourrait inliner, non bloquant

    Marquer les details comme non bloquants est une petite chose qui retire enormement de friction. L’auteur sait ce qu’il doit corriger pour fusionner par rapport a ce qui n’est que mon gout. Je prends aussi soin de dire quand quelque chose est vraiment bien. Une revue qui n’est que critique entraine les gens a redouter le processus.

    Relisez vite et finissez ce que vous commencez

    Une PR qui reste sans relecture deux jours bloque une personne et pourrit a mesure que main avance dessous. Je traite les demandes de relecture comme une priorite quasi en haut de ma file, idealement le jour meme. Le cout d’une PR a l’arret s’accumule : l’auteur change de contexte, puis doit tout recharger quand le retour arrive enfin.

    Quand je relis, j’essaie de donner tous mes retours en une passe plutot que de distiller des commentaires sur trois tours. Rien n’est plus demoralisant que tout corriger, etre relu, et decouvrir cinq nouveaux commentaires qui etaient visibles depuis le debut.

    L’auteur a aussi des devoirs

    Les revues vont plus vite quand l’auteur les rend faciles. Avant de demander une relecture, j’ecris une description qui explique ce qui a change et pourquoi, je laisse des commentaires en ligne sur mon propre diff pour pointer ce qui n’est pas evident, et je m’assure que la CI est au vert. Une bonne description peut diviser par deux le temps de relecture, car le relecteur ne reconstitue pas l’intention a partir du diff.

    • Enoncez le probleme, pas seulement la solution.
    • Signalez tout ce dont vous n’etes pas sur et sur quoi vous voulez un regard.
    • Incluez des captures ou un exemple de sortie pour tout ce qui touche l’utilisateur.

    Etre en desaccord, puis s’engager

    Parfois l’auteur et le relecteur voient simplement les choses autrement. Quand le desaccord porte sur le gout plutot que sur la justesse, je m’en remets a la decision de l’auteur apres avoir exprime mon avis une fois. Trainer une PR sur cinq tours pour une preference stylistique brule un capital de bonne volonte dont vous aurez besoin plus tard. Gardez les positions fermes pour ce qui compte vraiment : la justesse, la securite, et les contrats de donnees qui me tiennent tant a coeur dans le guide de conception d’API REST. Reussissez ceux-la et laissez filer les petites choses.

  • Bonnes pratiques de workflow Git et de branches

    Bonnes pratiques de workflow Git et de branches

    J’ai travaille dans des equipes qui traitaient Git comme un rituel sacre et dans d’autres qui le traitaient comme un tiroir fourre-tout. Aucun de ces extremes ne livre du bon logiciel. Apres des annees a nettoyer des historiques chaotiques et a demeler des catastrophes de fusion, j’ai adopte une serie d’habitudes qui gardent les choses calmes. Aucune n’est ingenieuse. C’est justement le but.

    Choisissez un modele de branches et cessez d’en debattre

    Le modele compte moins que l’accord. Pour la plupart des equipes produit, j’utilise le developpement sur tronc commun avec des branches de fonctionnalite a courte duree de vie. Vous partez de main, vous travaillez un jour ou deux, vous fusionnez, vous supprimez la branche. Plus une branche vit longtemps, plus elle derive, et plus la fusion finale devient penible. Les branches de version a longue duree ont leur place dans un logiciel qui sort a cadence fixe vers des clients qui ne peuvent pas se mettre a jour a la demande, mais pour une application web deployee plusieurs fois par jour, elles ne sont que du poids inutile.

    Ce que j’evite activement, c’est le montage GitFlow elabore ou develop, release, hotfix et feature s’entrelacent. J’ai vu ce systeme derouter les nouveaux pendant des semaines. Si votre deploiement est continu, vos branches devraient l’etre aussi.

    Ecrivez des commits qui expliquent le pourquoi

    Un message de commit est une note pour celui qui lira l’historique a 2h du matin pendant un incident, et cette personne pourrait etre vous. Le diff montre deja ce qui a change. Le message doit capturer pourquoi. Je garde la ligne de sujet sous environ cinquante caracteres, a l’imperatif, et j’utilise le corps pour expliquer le raisonnement quand le changement n’est pas evident.

    fix: empeche le double paiement lors d'une nouvelle tentative

Le client de paiement reessayait sur un 504 alors que
le debit etait deja passe cote passerelle. On clef
desormais la requete avec un jeton d'idempotence pour
que la passerelle deduplique. Ferme #482.

    Les commits atomiques sont l’autre moitie de l’affaire. Un changement logique par commit. Quand un commit fait trois choses sans rapport, vous ne pouvez jamais en annuler une seule proprement, et le bisect devient inutile. Si vous ecrivez « et » dans une ligne de sujet, ce sont deux commits.

    Rebasez votre propre travail, fusionnez le travail partage

    C’est la regle qui evite le plus de douleur. Avant d’ouvrir une pull request, je rebase ma branche sur la derniere version de main pour que mes changements reposent sur la realite actuelle et que la relecture soit fluide. Mais des qu’une branche est partagee ou qu’une PR est ouverte et que d’autres l’ont regardee, j’arrete de rebaser et je fusionne, car reecrire un historique publie oblige tous les autres a recuperer leur etat local.

    • Rebasez pour ranger vos commits locaux avant qu’ils soient publics.
    • Utilisez le rebase interactif pour ecraser les inevitables commits « correction de faute » et « vraie correction ».
    • Ne forcez jamais le push d’une branche sur laquelle d’autres construisent.
    • Protegez main pour que personne ne puisse y pousser directement.

    Gardez main toujours livrable

    La propriete la plus precieuse d’un depot, c’est que main fonctionne toujours. Si main est au vert, vous pouvez sortir une version a tout moment, et un deploiement casse se repare en annulant une seule fusion. J’impose cela avec des verifications obligatoires : les tests et le linting doivent passer avant meme que le bouton de fusion apparaisse. Cela rejoint directement ma facon de mener les relectures, que j’ai abordee dans les bonnes pratiques de revue de code. Un historique propre rend les relectures plus rapides, et de bonnes relectures gardent l’historique propre. Les deux se nourrissent mutuellement.

    Rendez l’annulation banale

    Quand quelque chose casse en production, l’action sure la plus rapide est en general d’annuler, pas de deboguer en direct. Ecraser chaque PR en un seul commit sur main rend cela trivial : une PR est un commit, et l’annuler retire toute la fonctionnalite proprement. J’aime les fusions ecrasees pour cette raison precise sur le code applicatif, meme si pour les bibliotheques ou l’historique de chaque commit a une vraie valeur, je conserve l’historique complet.

    Etiquetez vos versions pour toujours pouvoir repondre a « qu’est-ce qui tournait mardi dernier ». Une etiquette legere ne coute rien et transforme une question vague en reponse d’une ligne.

    Quelques habitudes qui paient discretement

    • Validez un .gitignore sense des le premier jour pour que secrets et artefacts de build n’entrent jamais dans l’historique. Retirer un identifiant fuite de l’historique gache un apres-midi.
    • Tirez avec rebase par defaut pour eviter le bruit des commits de fusion a chaque synchro.
    • Gardez des PR petites. Une PR de 200 lignes recoit une vraie relecture. Une PR de 2000 lignes recoit un tampon.

    Git recompense la discipline plus que le savoir. Vous n’avez pas besoin de memoriser les commandes de plomberie. Vous avez besoin d’un petit ensemble d’accords que tout le monde suit vraiment. Le meme raisonnement revient quand je concois des stockages de donnees, ce que j’ai couvert dans les bonnes pratiques de schema de base de donnees, ou quelques conventions fermes tot epargnent un nettoyage enorme plus tard.