Une mise à jour pour Omeka v3.

[jajm]

Hello Daniel,

J'ai regardé rapidement les modifications proposées et il y a plusieurs groupes de modification indépendants les uns des autres:

1. la mise à niveau du code pour Omeka 3.x
2. la vérification des prérequis à l'installation du module
3. le fix CSRF
4. l'amélioration de la doc (README)
5. l'incrémentation de la version du module
6. la modification du fichier .gitignore
7. la déclaration strict_types=1 + la déclaration des types de retour pour certaines méthodes

Chacun de ces points mériterait une pull request dédiée selon moi, ça faciliterait les échanges et les tests.

Sinon, quelques remarques/questions en vrac:

• le .gitignore a l'air d'être un .gitignore générique récupéré depuis un autre module. Du coup la moitié du .gitignore n'est pas utile en ce qui concerne ce module.
• Pourquoi PHP 7.3 est requis ?
• Pour le fix CSRF, j'ai l'impression qu'il manquait juste l'appel à init(), mais il y a beaucoup d'autres changements sur le formulaire de config, notamment la suppression du fichier .phtml. Pourquoi/est-ce que c'était nécessaire ? (et pourquoi le changement s/role/ldap_role/ ?)
• Dans le readme la cible du lien Ldap.zip n'existe pas
• Je préfère gérer l'incrémentation du numéro de version en dehors des pull request : il faut aussi mettre à jour le changelog (ou le créer dans le cas présent), poser un tag, construire le .zip, créer la release github, ... Bref, c'est un processus à part

[Daniel-KM]

Bonjour @jajm ,

J'ai automatisé la mise à niveau des modules et de mon point de vue il est plus facile de faire des "cherry-pick" que de multiplier les "pull requests" sur github. Je peux bien sûr faire un pr par commit ou encore diviser les commits. D'autres préfèrent un commit global, d'autres préfèrent un commit pour chaque micro modification; c'est essentiellement un choix.

Effectivement :

• la mise à niveau du code pour Omeka 3.x => 1er commit
• la vérification des prérequis à l'installation du module => nécessaire si on conserve "^2.10" qui renvoie désormais laminas/ldap 2.11, qui n'est plus compatible php 7.1 ni 7.2. On peut forcer 2.10 sinon.
• le fix CSRF: on peut effectivement diviser en trois commits : correction, simplification (le fichier phtml est inutile puisqu'il n'y a pas de texte ; et il est plus simple d'avoir les mêmes noms dans le formulaire et dans la base), sécurité (il faut vérifier tous les retours de formulaires).
• l'amélioration de la doc (README) => c'est plus clair quand tout les détails techniques sont indiqués dans la doc plutôt que dans le code ; j'ai vu d'ailleurs qu'il n'y a pas de livraison, dans ce cas il faut supprimer le texte.
• l'incrémentation de la version du module => le commit peut être supprimé ; et pour la publication tag, zip, création de la livraison, j'ai aussi automatisé tout cela (cf. omeka/omeka-s#1159)
• la modification du fichier .gitignore => c'est lié à l'automatisation de la mise à niveau et avoir le même gitignore dans tous les modules m'évite d'y réfléchir chaque fois
• la déclaration strict_types=1 + la déclaration des types de retour pour certaines méthodes => autant profiter des fonctionnalités de php qui permettent d'améliorer la qualité du code

Je préparerai 6 pr dans quelques jours si besoin.

[jajm]

* la vérification des prérequis à l'installation du module => nécessaire
si on conserve "^2.10" qui renvoie désormais laminas/ldap 2.11, qui n'est
plus compatible php 7.1 ni 7.2. On peut forcer 2.10 sinon.

Avec https://getcomposer.org/doc/06-config.md#platform on peut forcer la version de php pour la résolution des dépendances. C'est ce que fait Omeka et c'est ce que j'ai fait aussi ici

* le fix CSRF: on peut effectivement diviser en trois commits :
correction, simplification (le fichier phtml est inutile puisqu'il n'y a
pas de texte ; et il est plus simple d'avoir les mêmes noms dans le
formulaire et dans la base), sécurité (il faut vérifier tous les retours
de formulaires).

Ok, j'ai intégré le commit tel quel

* l'amélioration de la doc (README) => c'est plus clair quand tout les
détails techniques sont indiqués dans la doc plutôt que dans le code ;
j'ai vu d'ailleurs qu'il n'y a pas de livraison, dans ce cas il faut
supprimer le texte.

J'ai repris une partie du README.

* la déclaration strict_types=1 + la déclaration des types de retour pour
certaines méthodes => autant profiter des fonctionnalités de php qui
permettent d'améliorer la qualité du code

J'ai pas encore repris ça, mais je le ferai dès que j'ai le temps.

J'ai fait plusieurs releases Github (la dernière est compatible Omeka 3.x) et j'en ai profité pour déclarer le module sur omeka.org

Merci pour la pull request!