Rupture sauvage de compat sur `saisies_modifier()` #216

Closed
opened 9 months ago by cerdic · 6 comments
cerdic commented 9 months ago
Owner

C'est une histoire en 2 commits sur la fonction saisies_modifier()

D'abord le 21 mars 2021, on prend en charge le nouveau_type_saisie à la racine au lieu d'être dans options et en signalant que "ça disparaitra" en 4.0

ae4a71c896

Et puis bon le 8 mai 2021 on vire le support de compat sur l'ancienne position
e0c6472776

en mentionnant "une compatibilite historique qu'on supprime"

"une compatibilite historique" qui date d'il y a 2 mois c'est quand même fort de café, et bon du coup paf, le code des plugins perso pète tout à fait gratuitement - je ne sais même pas si il y a une quelconque raison technique ou un avantage à déplacer cette option...

C'est pas vraiment ce que j'appelerai un développement smooth pour les utilisateurs...

C'est une histoire en 2 commits sur la fonction `saisies_modifier()` D'abord le 21 mars 2021, on prend en charge le `nouveau_type_saisie` à la racine au lieu d'être dans `options` et en signalant que "ça disparaitra" en 4.0 https://git.spip.net/spip-contrib-extensions/saisies/commit/ae4a71c896a759bd4876b2fc01d99f905e22000a Et puis bon le 8 mai 2021 on vire le support de compat sur l'ancienne position https://git.spip.net/spip-contrib-extensions/saisies/commit/e0c6472776ea08e07ffb43d7b253a8acf96f1524 en mentionnant "une compatibilite historique qu'on supprime" "une compatibilite historique" qui date d'il y a 2 mois c'est quand même fort de café, et bon du coup paf, le code des plugins perso pète tout à fait gratuitement - je ne sais même pas si il y a une quelconque raison technique ou un avantage à déplacer cette option... C'est pas vraiment ce que j'appelerai un développement smooth pour les utilisateurs...
Poster
Owner

je vois bien que l'idée c'est d'avoir introduit le support de la nouvelle syntaxe dans la v3 puis de virer l'ancienne syntaxe dans la v4 du plugin, mais franchement tout ça avec un overlap de 2 mois et alors même que la nouvelle syntaxe est apparue en loucedé dans une version existante ça me parait assez léger...

je vois bien que l'idée c'est d'avoir introduit le support de la nouvelle syntaxe dans la v3 puis de virer l'ancienne syntaxe dans la v4 du plugin, mais franchement tout ça avec un overlap de 2 mois et alors même que la nouvelle syntaxe est apparue en loucedé dans une version existante ça me parait assez léger...

Pour comprendre, je ne me rappelle même plus déjà pourquoi ça a été déplacé à la racine, je ne retrouve pas encore dans les tickets/PR où ça a été discuté.

Pour comprendre, je ne me rappelle même plus déjà pourquoi ça a été déplacé à la racine, je ne retrouve pas encore dans les tickets/PR où ça a été discuté.
Poster
Owner

ah oui ça je sais pas...

ah oui ça je sais pas...
Owner

Concernant la suppression, elle fait partie de la liste des élagages que j'avais proposés concernant le passage en 4.0 #10

a l'époque, j'avais demandé l'avis des gens concernant toutes ces suppressions en 4.0, et obtenu l'avis positif de Rastapopoulos (via IRC). Et donc c'était annoncé que cela sauterait en 4.0

Quant au choix de ne plus mettre cela dans options : et bien effectivement je ne retrouve plus de traces en tickets. Mais cela avait été discuté via IRC, lorsqu'on réflechissait à la syntaxe concernant les héritages / modification de saisies. L'idée est qu'une information de transformation n'est pas une option de la saisie, mais bien une option de la transformation.

Maintenant, si vous tenez on peut reverter le second commit, mais bon, ne disons pas que les choses n'avaient pas été annoncés.

Concernant la suppression, elle fait partie de la liste des élagages que j'avais proposés concernant le passage en 4.0 https://git.spip.net/spip-contrib-extensions/saisies/issues/10 a l'époque, j'avais demandé l'avis des gens concernant toutes ces suppressions en 4.0, et obtenu l'avis positif de Rastapopoulos (via IRC). Et donc c'était annoncé que cela sauterait en 4.0 Quant au choix de ne plus mettre cela dans options : et bien effectivement je ne retrouve plus de traces en tickets. Mais cela avait été discuté via IRC, lorsqu'on réflechissait à la syntaxe concernant les héritages / modification de saisies. L'idée est qu'une information de transformation n'est pas une option de la saisie, mais bien une option de la transformation. Maintenant, si vous tenez on peut reverter le second commit, mais bon, ne disons pas que les choses n'avaient pas été annoncés.

Je suppose que ce que voulais dire @cerdic c'est qu'un truc annoncé 1) dans un commentaire du code, 2) dans un ticket ou jalon lu juste par les mainteneurs du plugin + 3) avec juste 2 mois entre l'annonce déjà semi-cachée et la suppression finale… bah c'est un peu comme si ça n'avait jamais été annoncé du tout pour les utilisateurs finaux de l'api.

En fait le cas le plus courant, c'est que quand on décide de déprécier une fonctionnalité, c'est le fait d'être pour l'instant seulement dépréciée qui est releasé (et donc annoncé) dans la version suivante N+1. Puis cette déprécation reste un moment. Puis la fonctionnalité est retirée définitivement en N+2. Alors que là on a mis (relativement caché) que c'était déprécié directement dans la version N du moment, puis ça a été supprimé en N+1 (très) peu de temps après. C'est vouloir aller trop vite.

Il faudra qu'on soit plus attentif (je m'inclus bien sûr dedans) à ça pour les prochains nettoyages, déprécations, suppressions.

Sinon, vu que le fallback pour chercher dans l'ancienne manière ne coûte rien, éventuellement ça pourrait être remis… si c'est ce que demande @cerdic (vu que le ticket ne demande rien et parle d'une release d'il y a 7 mois ou plus)

Je suppose que ce que voulais dire @cerdic c'est qu'un truc annoncé 1) dans un commentaire du code, 2) dans un ticket ou jalon lu juste par les mainteneurs du plugin + 3) avec juste 2 mois entre l'annonce déjà semi-cachée et la suppression finale… bah c'est un peu comme si ça n'avait jamais été annoncé du tout pour les utilisateurs finaux de l'api. En fait le cas le plus courant, c'est que quand on décide de déprécier une fonctionnalité, c'est *le fait d'être pour l'instant seulement dépréciée* qui est releasé (et donc annoncé) dans la version suivante N+1. Puis cette déprécation reste un moment. Puis la fonctionnalité est retirée définitivement en N+2. Alors que là on a mis (relativement caché) que c'était déprécié directement dans la version N du moment, puis ça a été supprimé en N+1 (très) peu de temps après. C'est vouloir aller trop vite. Il faudra qu'on soit plus attentif (je m'inclus bien sûr dedans) à ça pour les prochains nettoyages, déprécations, suppressions. Sinon, vu que le fallback pour chercher dans l'ancienne manière ne coûte rien, éventuellement ça pourrait être remis… si c'est ce que demande @cerdic (vu que le ticket ne demande rien et parle d'une release d'il y a 7 mois ou plus)
Poster
Owner

moi j'ai modifié mon code perso pour le refaire marcher mais

  • de fait on a une rupture de compat brutale entre la v3 et la v4 (et qui est une mise à jour forcée pour passer de SPIP 3.2 à SPIP 4.x)
  • c'est annoncé nulle part (ni dans un changelog, ni dans une documentation)

Donc t'as aucune chance d'éviter le bug même en étant plein de bonne volonté.

Je pense en effet que le mieux serait de garder le patch de compat, en indiquant dans le code que c'est déprécié, et en générant un log pour donner une chance aux gens de savoir qu'ils font un truc déprécié et que ça va bientôt casser...

Mais aussi il faut que les ruptures de compat (effectives ou prévues) figurent clairement quelque part, surtout sur une brique de base comme saisies sur laquelle plein d'autres plugins reposent...

Même a minima on préfèrerait que l'utilisation de cette ancienne option non supportée génère une erreur_squelette() avec un message : au moins c'est clair, tu as une erreur, tu sais quoi corriger.

Là c'est juste que silencieusement ça marche plus, et si tu fais pas un examen détaillé de ton interface après migration (et encore si tu as la chance que ce soit dans un bout d'interface directement visible) tu rates totalement le bug

moi j'ai modifié mon code perso pour le refaire marcher mais * de fait on a une rupture de compat brutale entre la v3 et la v4 (et qui est une mise à jour forcée pour passer de SPIP 3.2 à SPIP 4.x) * c'est annoncé nulle part (ni dans un changelog, ni dans une documentation) Donc t'as aucune chance d'éviter le bug même en étant plein de bonne volonté. Je pense en effet que le mieux serait de garder le patch de compat, en indiquant dans le code que c'est déprécié, et en générant un log pour donner une chance aux gens de savoir qu'ils font un truc déprécié et que ça va bientôt casser... Mais aussi il faut que les ruptures de compat (effectives ou prévues) figurent clairement quelque part, surtout sur une brique de base comme saisies sur laquelle plein d'autres plugins reposent... Même a minima on préfèrerait que l'utilisation de cette ancienne option non supportée génère une `erreur_squelette()` avec un message : au moins c'est clair, tu as une erreur, tu sais quoi corriger. Là c'est juste que silencieusement ça marche plus, et si tu fais pas un examen détaillé de ton interface après migration (et encore si tu as la chance que ce soit dans un bout d'interface directement visible) tu rates totalement le bug
maieul closed this issue 6 months ago
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.