Séparer ses commits pour mieux piger
Quand on parle de code et de découpage, on comprend vite les intérêts suivants :
- Maintenabilité
- Simplification
- Responsabilité des méthodes
- Lisibilité
- …
Quand on parle de commit, je constate souvent que les intérêts de les séparer ne sont pas aussi clairs.
Ici nous allons aborder l’une des nombreuses raisons de l’avantage de séparer ses commits afin d’aider la compréhension générale de l’équipe vis à vis du code produit.
La relecture de code
Si vous travaillez en équipe, avec Git, et avec des merge request (ou pull request), vous avez surement déjà été confronté au fameux commit “fourre tout”, ou aux commits de “correction des retours suite à la revue de code”. Si ce n’est pas le cas, je ne suis pas certain de vous apporter quelque chose aujourd’hui, sinon, restez un petit peu pour mieux piger !
Personnellement je me suis déjà confronté à ce genre de situation. Nous fonctionnons en revue de code asynchrone depuis un moment et j’ai mon petit rituel quand je me connecte, je vais voir les pull requests en cours à relire. Je me plonge dans la compréhension de ce que le développeur a voulu faire, j’essaie de comprendre pourquoi ce code est fait comme ci ou comme ça, et j’essaie de relier tout cela à un besoin fonctionnel que je ne connais pas forcement. Tout cela me prend un certain temps si je veux le faire de manière assidue, et étant dans une équipe qui gère plusieurs projets, j’ai par moment beaucoup de pull request à relire, je n’ai pas spécialement ma journée à dédier à cela donc je dois faire des choix.
Alors vous allez me demander ce que viennent faire les commits là dedans, après tout, je vais relire tout le code donc ce n’est pas très grave si tout cela est dans un seul commit de 150 fichiers, intitulé :
“feat(la_feature): répond au besoin fonctionnel”
C’est pas faux, mais c’est aussi se placer en tant que celui qui a fait le développement, qui est dans son sujet et qui comprend l’intérêt. Mais faire relire son code c’est aussi avoir besoin d’un retour des autres pour s’améliorer ou simplement valider le fait qu’on ne laisse rien passer de critique qui pourrait compromettre la production.
Racontes moi une histoire
Et donc nous y voilà ! Comme ce sont les autres qui vont relire notre code, il est important de leur raconter une histoire. Il faut qu’à travers l’historique des commits de la pull request ils puissent comprendre ce que nous avons voulu faire, il faut leur permettre de comprendre notre cheminement.
Ce qui est fort, c’est que cela vaut pour les revues de code asynchrone, là où le développeur n’est pas là pour nous expliquer en live ce qu’il a fait et pourquoi, mais cela vaut également pour les revues de code synchrone ! En effet, cela permet au développeur de préparer sa revue de code et d’avoir un fil rouge à suivre pour présenter son travail !
Alors comment on fait ça ? C’est souvent ici que cela coince pour beaucoup, et ça se comprend ! Je vais essayer de vous montrer comment je fais pour mieux découper, sachant que comme toute pratique, c’est à force de pratiquer que l’on s’améliore, alors n’ayez crainte, cela viendra.
On va prendre un exemple avec une pull request qui contiendrait ce type de modifications :
- des mises à jour de packages
- une mise à jour de service
- la création d’une classe utilitaire
- l’utilisation de cette classe
Le découpage
build(packages): mise à jour des dépendances du projet
La lecture d’une mise à jour de package peut se ranger dans un commit à lui seul, ainsi je concentre mon attention exclusivement sur ça. Je peux donc surveiller les versions utilisées sans me polluer avec tout le reste du code fonctionnel.
build(services): mise à jour des services
Même constat avec la mise à jour d’un service par exemple, je vais ranger cette modification dans un seul commit afin de seulement voir la mise à jour de ce dernier, vérifier les propriétés qui ont changées, et garder cela en tête pour la suite de la lecture. Cela m’évite encore une fois de passer du coq à l’âne, il parait logique que le développeur ait commencé par mettre à jour un service afin d’utiliser les nouvelles propriétés par la suite. Il est donc plus simple de lire cela en premier plutôt que de se poser la question au fil de l’eau sur pourquoi telle ou telle propriété vient de changer dans mes couches applicatives.
feat(utils): création de ma classe utilitaire qui fait ce truc chouette (à décrire dans la vraie vie)
J’arrive maintenant à du concret, du code que j’ai produit, et là, personnellement je découpe par ordre. Si je dois créer une classe, une factory ou quelque chose qui à vocation à être utilisé ensuite, et bien je vais le ranger dans un commit à part, afin de focus la lecture uniquement sur ça, et au même titre que pour les modifications de services, avoir en tête ce qui vient d’être produit et qui sera utilisé par la suite.
feat(implémentation): utilisation de ma classe utilitaire dans tel bout de code
Enfin, dans notre exemple, je crée un commit pour montrer l’utilisation de ce que je viens d’écrire. On se concentre ici sur l’intégration et on peut vérifier la cohérence.
Syndrome du Wémé
“Oué mais” Geo, les tests là dedans ?! Pour simplifier je n’ai pas parlé de tests mais là il y a plusieurs écoles. Personnellement je pars du principe que c’est soumis à bonne conscience du développeur. Si on à réellement compris l’intérêt de découper ses commits pour faciliter la lecture, la question des tests ne se pose même pas.
Si j’écris un scénario traversant, je pense souvent qu’il à sa propre place dans un commit à lui afin de forcer le relecteur à comprendre le scénario et ce qu’il doit faire. Ainsi on commence par lire le test, chose sur laquelle on se concentre le moins en temps normal dans ce que j’ai pu voir. On comprend ce que l’application est censée faire et je peux appréhender la suite avec une meilleure compréhension du besoin.
Si je fais un simple petit test unitaire suite à une modification minime dans une classe, il n’y a parfois pas de raison de séparer les deux. Il n’y a pas de limites à suivre sur des quantités de lignes ou de fichiers pour séparer, c’est surtout à vous de trouver les bonnes jauges, séparer pour regrouper de manière logique et trouver les bons curseurs qui vous permettrons de faciliter les relectures de code et les rendre plus efficaces et surement gagner en qualité.
Autre astuce
Il existe surement des légendes en ce monde qui arrivent à tout bien faire en une seule fois et séparer tout de chaque ligne de code dans des commits au fil de l’eau sans jamais avoir besoin de revenir dessus. #respect
Bon moi je suis loin d’être une légende, et du coup j’aime bien faire tout mon développement, refactoriser des choses, optimiser, arrondir les angles etc, bref j’essaie de bien faire autant que possible. Mais du coup ce n’est pas vraiment pratique de commiter au fil de l’eau car il faut souvent “fixup” ou “rebase edit” et ce n’est pas spécialement un gain de temps en plein milieu d’un développement.
Donc je ne me prend pas trop la tête avec tout ça au départ. Ni durant tout le processus de développement à vrai dire. Je fais mon travail, je vérifie tout, on pair test, je fais les retouches. Si cela dure des jours, je n’ai pas honte de dire que je mets tout dans un commit “WIP” et je push. Ainsi, quand j’ai terminé, avant de lancer une pull request, je fais un git reset, et je reparcours toutes mes modifications pour les ranger dans des commits. Cela me permet de refaire une passe sur ce que j’ai fait, de préparer une possible revue de code synchrone, et vérifier que je n’ai rien oublié.
Histoire de clôturer tout cela, j’essaie de toujours garder en tête que bien écrire un commit c’est comme bien écrire du code. Il faut qu’il soit lisible et compréhensible par tous. Rien qu’à la lecture du titre je dois déjà avoir une idée de ce que je vais trouver dedans. Comme je le fais quand j’écris une méthode, mes commits doivent avoir une seule responsabilité, je trouve que c’est un essentiel pour garder l’historique simple et compréhensible.
Au final, et au risque de me répéter, il faut pratiquer, ne pas baisser les bras, et faire des erreurs pour apprendre et s’améliorer ! Et puis l’erreur de nommage n’a pas d’impact dans la finalité du code, c’est à vous et votre équipe de définir ce qui vous convient le mieux. Cela peut paraitre overkill, mais au final on parle de quelque chose qui ne prend pas plus de 5 minutes à faire et qui va vous faire gagner un qualité et efficacité de relecture. Un faible investissement pour, je pense, un énorme gain, sans parler de tous les autres avantages que cela peut apporter dans d’autres processus du projet : génération de changelog, investigation, rollback, cherry-pick et j’en passe.
Le mieux reste encore d’essayer de vous convaincre par vous même :D
