Proposition d'évolution des règles de contribution

Salut,

Maintenant qu’on a de beaux changelogs dans les dépôts du core et de ses plugins cf Sprint relecture de PRs avant release - #3 par marcimat je propose d’ajouter une précision à nos règles de contribution, dans l’entrée 3, afin de mentionner qu’une PR doit aussi comporter l’ajout d’une ligne dans le changelog du dépôt.

Quelque chose du type :

De plus, il faut bien veiller à ajouter une ligne dans la fichier CHANGELOG du dépôt afin de décrire clairement et de manière concise les modifications apportées par la PR.

Vos avis ?

1 « J'aime »

Oui clairement.

Oui bonne idée

Pour info, on est en pause sur le sujet car l’adoption de la norme Conventional Commits pourrait nous permettre de générer les changelog de manière automatisée cf https://git.spip.net/spip/spip/issues/5208

À suivre…

Pour donner suite, je pense qu’il faut oublier la proposition initiale, car l’ajout de la ligne au changelog dans une PR conduit souvent à des conflits de merge si le changelog a évolué entre temps.

Par contre, il semble qu’on soit bien en train d’adopter la norme Conventional Commits, et je pense qu’il est temps de le préciser dans nos règles de commit.

Voici ma proposition d’ajout à Contribuer au développement de SPIP - SPIP

Au point 3 de la liste :

Les messages des commits de la branche issue_xxx doivent suivre la nomenclature des Commits Conventionnels. Le corps du message doit être clair et explicatif : décrire le problème traité et les évolutions ou corrections apportées. De plus, il faut bien veiller à ajouter une ligne dans la fichier CHANGELOG du dépôt dans un commit à part afin d’éviter les conflits lors de la fusion. Pour finir, la série de commits doit être linéaire (sans commits de merge).

Puis, à la fin de ce bloc « Règles de contribution » :

À propos des Commits Conventionnels et des fichiers CHANGELOG

La liste des types disponibles pour les commits conventionnels est la suivante : build, change, chore, ci, deprecate, doc, feat, fix, perf, remove, revert, security, style, test, i18n (détails sur git.spip.net). Le pied de page du message doit contenir une référence au ticket lié à la PR sous la forme Fix: #XXX. Exemple de log de commit complet :

fix: réparer le lien vers la licence GPL dans le pied des pages du privé

en le remplaçant par un simple lien vers le site officiel gnu.org

Fix: #5328

Les fichiers de CHANGELOG suivent la norme Tenez un Changelog. Chaque entrée du changelog doit :

  • référencer le ticket (ou la PR s’il n’y a pas de ticket correspondant)
  • décrire clairement et de manière concise les modifications apportées par la PR (pour cela, vous pouvez simplement reprendre la première ligne du message de commit)

Exemple d’entrée de changelog correspondante au log de commit précédent :

- #5328 Réparer le lien vers la licence GPL dans le pied des pages du privé


Dites moi si ça vous va ou si c’est plus pratique que je colle ça direct dans l’article pour faciliter la relecture.

1 « J'aime »

Je vote pour !

2 remarques :

  1. Il faudrait une mention à propos des scopes : dire s’il y a une liste prédéfinie recommandée, ou si on accepte tout. C’est toujours pas hyper clair pour moi, même après lecture de la doc (qui renvoie elle-même ailleurs).
  1. Ou Refs aussi. Fix déclenche automatiquement la fermeture du ticket quand la PR est fusionnée si je me trompe pas ? Donc indiquer les 2 possibilités.

En pratique, ça se gère comment s’il y a un conflit qui interdit de créer la PR ? (Ou de la merger avec le master entre le moment de la création de la PR et sa validation)

la PR peut normalement toujours être créé, le problème est plus le merge.

Mais du coup : avant de merger tu rebase le master sur la branche, et tu résoud les conflits manuellement (pas possible d’automatiser cela, par définition, et tu aurais eu le même problème avec un commit de merge). Puis tu push --force-with-lease la branche.

En gros dans git tu as deux manières de rattraper des divergences entre branches : les merge et les rebase. Chacun et chacune on des avantages et des inconvénients. Mais le principe de base, c’est que si tu as une branche X qui est en retard sur la branche principal (master/main), tu rebase master/main dans la branche X ce qui permet de faire comme si X avait commencé plus tard à diverger de master/main (c’est d’ailleurs le même topo si jamais ta version local est divergente de ta version distante).

Globalement dans SPIP on a tendance à refuser les commits de merge, qui pourrait cependant se justifier dans certains cas (notamment lorsqu’on a une grosse branche avec plein de commits fonctionnelles), mais c’est une question de convention.
Une ressource que je trouve pas degeu pour comprendre les différences.

1 « J'aime »

Comme dit tcharles, il faut aussi Refs:, et pas uniquement Fix: dans le pied… c’est surtout là pour faire la liaison avec le ticket.

Sinon, RAS, à part qu’il faudra certainement développer le détail des types aussi quelque part.

PEC, je propose :

Le pied de page du message doit contenir une référence au ticket lié à la PR sous la forme de Refs: #XXX ou Fix: #XXX pour fermer le ticket automatiquement lors de la fusion.

Pour les scopes je n’ai pas détaillé car il n’y a pas de liste à ce jour, ça pourrait par exemple être la liste des tags dispos pour les tickets, mais pas certain que ça couvre tout, autant ouvert pour l’instant amha.

Je peux ajouter une mention à ce sujet, mais j’ai peur d’alourdir le bouzin, je vais voir si je peux faire concis.

:+1:

Proposition :

La liste des types disponibles pour les commits conventionnels est la suivante : build, change, chore, ci, deprecate, doc, feat, fix, perf, remove, revert, security, style, test, i18n (détails sur git.spip.net). Le type peut éventuellement être complété par un scope entre parenthèses afin de préciser le composant ou la fonctionnalité modifiée (exemples avec les commits 13a90d3878 et 5512ce374b).

Et voilà, j’ai reporté les modifications dans l’article.

2 « J'aime »