Composerisation des plugins de spip-contrib-extensions

Dev
,
1

Bonjour à toustes,

Je crée ce fil de discussion suite à la proposition de @JamesRezo sur une demande de fusion.

J’ai fait quelques PR de ce genre hier soir et ce matin, l’idée étant essentiellement pour moi d’ajouter les infos nécessaires dans des composer.json afin de rendre les plugins en question installables via spip-league/composer-installer pour pouvoir gérer facilement des déploiements-types de SPIP avec Composer.

J’en ai profité pour ajouter et appliquer les outils utilisés ailleurs pour appliquer les règles de codage (Rector[1] via spip-league/rector, ECS via spip-league/easy-coding-standard, et PHPStan).

Je voulais donc savoir si c’était OK pour vous que je continue à vous inonder de PR comme ça, s’il vous plaît, ou s’il valait mieux que je fasse autrement (quitte à ne faire que des PR avec un composer.json minimal extrait de paquet.xml, par exemple, ou à harmoniser autrement les outils utilisés, etc.).

Merci ! :slight_smile:

++
Glop

  1. Je viens d’ailleurs de me rendre compte qu’il y a actuellement un bug dans la règle LogicalToBooleanRector de Rector (v2.5.4) et qu’il produit du code qui n’est pas équivalent au code en entrée :sob: Je vais revérifier toutes mes PR pour m’assurer que je n’ai pas introduit de bugs, du coup… ↩︎

2

Moi ça me va très bien, merci :slight_smile:

1 « J'aime »
3

Moi aussi, mais je pense qu’il faut pas se précipiter :wink:

Comme dit ici, je pense qu’on pourrait envisager une nouvelle orga spip-contrib dans laquelle on déplacerait les plugins communautaires composerisés et mettre en place la CI ainsi que la mise à jour du dépôt composer de SPIP.

Pourquoi ne pas les laisser dans la vieille orga ?

Parce qu’il y a environ un millier de plugins et qu’il ne sont pas tous destinés à être composerisés, que certains mainteneurs ont leur mot à dire, etc. quant aux règles de coding, sur la composerisation elle-même, etc. De plus, évitons aussi se précipiter pour les aspects mainteneureuses/développeureuses

Donc, avant de pouvoir le faire, faut qu’on trouve un terrain d’entente avec des personnes qu’on a du mal a identifier parce que plus là ou peu actif, ou très discret…

Ça va prendre du temps. Et si pendant ce temps, on accumule des PR, certes similaires, mais en très grand nombre, avec des risques de multiplication de cas particuliers, de querelles ou tout autre débat, interrogration, c’est un travail qui va s’enliser dans le temps et les profondeurs des archives…

Toutefois, merci @glop de t’y coller, c’est enthousiasmant. Mais je pense qu’on devrait faire aboutir ce chantier avec un nombre maîtrisable de plugins.

4

Salut et merci pour vos retours ! :slight_smile:

Pour ma part, j’envisageais de composeriser en tout une cinquantaine de plugins, qui correspondent à ceux que je déploie dans les installations que je fais.

En ce qui me concerne, l’objectif premier est qu’il existe au moins quelque part pour chaque plugin dont j’ai besoin une branche avec un composer.json contenant les infos nécessaires pour pouvoir en faire un paquet installable avec spip-league/composer-installer (soit, a minima, name, type, et éventuellement require).

Du coup, comme je me dis que ça peut profiter à d’autres, j’en profite pour en faire une PR avec quelques trucs en plus (infos du composer.json extraites de paquet.xml, ainsi que ECS, Rector, PHPStan), mais rien de tout ça n’est obligatoire si vous pensez que c’est superflu.

Aussi, pour l’instant, je tente de faire un truc minimal et à peu près uniforme (mêmes dépendances Composer, mêmes fichiers ecs.php, rector.php et phpstan.neon.dist, repris d’autres plugins). Je n’ai pas mis de CI en place car je ne savais pas trop quelle était la pratique, mais c’est aussi envisageable, bien sûr.

Et si la forme PR ne convient pas, je peux aussi forker les plugins en question sur mon espace perso du Gitlab (ça me fera toujours des branches utilisables), voire sur une forge privée pour ne pas encombrer ici :slight_smile:

Bref, voilà, vous me dites ! Merci, en tout cas !

++
Glop

5

50 / 1000 ça me paraît raisonnable :slight_smile:

ecs et phpstan ne sont pas superflus, du coup, ajouter la config rector dans la foulée n’est pas gênante. Enfin à mon sens …

La CI, on n’en fait pas une obligation, mais c’est tellement pratique pour les PR suivantes. Ceci dit, ça peut être un autre chantier, plus tard, pas d’inconvénient.

Je ferai le job de la mise à disposition par le dépôt Composer de SPIP au fur et à mesure, a priori.

6

Super, merci beaucoup ! :slight_smile:

Pour la CI, comme tu veux, mais je peux aussi la rajouter aux PR existantes et à venir, en reprenant par exemple le modèle d’un plugin de spip/*.

Et trop bien pour les ajouts au dépôt Composer de SPIP, merci ! :heart:

++
Glop

7

Attention sur ce point, il ne faut pas copier « brutalement », comme je te le disais sur Composerisation, Rector, ECS et PHPStan (!1) · Requêtes de fusion · spip-contrib-extensions / badge_don · GitLab les files à coller dans autoload-dev dépendent de l’usage des fonctions du core/sdk dans le plugin :wink:

8

Oui, carrément, merci beaucoup ! Et pardon, c’est moi qui n’avais pas compris ta remarque sur la PR en question ! :slight_smile:

9

Aussi, je ne sais pas si je le pose ici ou si je fais un fil à part, mais pour info, Rector introduit parfois des bugs dans le code, notamment quand il réécrit les and et or en && et ||, respectivement. En moins de 48 h d’utilisation, je viens d’identifier 3 cas différents (#9797, #9799 et #9800), et qui concernent des constructions PHP souvent présentes dans le code de SPIP (comme des affectations de variables dans les conditions des if).

Je ne sais pas dans quelle mesure Rector a été utilisé de manière automatique (sans vérifier les changements qu’il effectuait), ni sur quelle partie du code, mais ça m’a l’air de pouvoir facilement créer des bugs bien pénibles à trouver :confused:

(Heureusement, le dev principal de Rector est ultra réactif et corrige les bugs plus vite que son ombre ! :heart:)

++
Glop

10

Oui, en effet.

Nous les corrigeons dans SPIP5 mais nous les avons laisser dans SPIP4. Ceci traduit en config Rector : on supprime la règle ci-dessous pour le code compatible SPIP4.4

// rector.php

// ...

return RectorConfig::configure()
-    ->withRules([LogicalToBooleanRector::class])
;

idem, on corrige autant que faire se peut cette pratique dans SPIP5

Oui, il semble impossible à suivre :slight_smile: et parfois, ça nous fait des misères, comme récemment sur ecs.

1 « J'aime »
11

Ah oui, super, je n’avais pas remarqué ces différences de pratiques de code entre SPIP 4 et SPIP 5 !

Merci beaucoup pour les explications :slight_smile:

12

Pour celles et ceux qui ne connaissent pas Composer et qui veulent s’informer, une publication de @JamesRezo sur blog de SPIP qui permet d’appréhender les enjeux de cet outil.

1 « J'aime »