Bonjour,
Je trouve que la meilleure doc c’est celle qui est dans le code donc les commentaires et aussi surtout la phpdoc
Pas pour l’utilisateur bien sûr, mais pour tous les développeurs (plugin ou core) on a toujours besoin de faire référence au code, et le code jeedom manque cruellement de commentaires.
J’ai fait un test, mais il faut le prendre comme il est, pour tester les balises et la génération du site: conversion des commentaires au format phpdoc, en un site statique html. Comme ce n’est qu’un test j’ai juste fait la classe eqLogic, et encore j’imagine que mes commentaires ne sont pas pertinent mais je connais mal ce code encore. https://pifou25.github.io/jeedom-core/classes/eqLogic.html
l’original pour comparaison https://doc.jeedom.com/dev/phpdoc/4.1/classes/eqLogic.html
Je l’ai généré avec phpdocumentor, par contre je patauge encore avec docker les github action et les circle-ci automatisés.
Super ça !
Une vrai doc php commentée avec des exemples serait tellement utile…
Rien que de savoir ce que l’on peut / doit passer en paramètre à une méthode …
j’ai mis le tag @package bien qu’il n’y a pas de namespace à ce jour, mais on y viendra sûrement un jour. Le tag dans phpdoc n’a aucune influence sur le code mais ça permet de ranger les classes comme avec un namespace lorsqu’il n’y en a pas.
j’utilise de plus en plus les tags @see et @uses pour faire des liens entre les différentes portions de code, c’est très pratique pour naviguer. Je crois que certains IDE prennent en compte ces tags aussi pour la navigation dans le code, si certains ont des retours ?
j’utilise aussi le tag @link pour ajouter des liens vers la doc officielle jeedom
cette classe cmd.class.php est outrageusement longue, il faudra la diviser en plein de petites du coup, les descriptions que j’ai pu mettre sont surement très approximatives, je m’en excuse, mais je découvre encore le code.
J’ai mis en place une génération automatique (avec circleci): à chaque push sur la branche phpdoc ça génère et déploie le résultat tout seul (plus d’info si ça vous intéresse?)
Pour finir, je vous invite à participer sur ma branche phpdocs pour les classes qui sont dans core/class - je ne pense pas que c’est intéressant de faire les autres fichiers.
Bon, c’est l’échec: à mon retour de vacances, je n’arrive plus à aligner ma branche sur la branche d’origine (upstream) il y a eu trop d’écart, des conflits dans tous les sens…
Vous faites comment pour gérer les merge / rebase sur des fichiers de +3000L et 50 conflits?
Je me permet de faire revivre ce post initié par @pifou sur l’amélioration de la documentation PHPDoc du core, je souhaite relancer ce sujet avec une approche légèrement différente.
Pour donner du contexte, j’ai déjà envoyé une PR qui documentait la class ajax.class.php dans le but principal d’ouvrir la discussion sur la façon d’améliorer la phpdoc. Après un commentaire élogieux de @pifou (merci à lui) cette PR a été mergée directement… puis une demande de revert a été faite par @Salvialf qui a finalement donné lieu a discussion, et donc à up ce post.
Voici l’approche que j’ai décidé de prendre lorsque j’ai fais cette PR :
Documentation classe par classe
Commencer par des classes plus petites et indépendantes
Pour les classes plus grosse, les faire par morceaux/groupe de méthodes
Soumission de PR individuelles pour chaque classe
Focus sur la documentation sans modification du code
Standards de Documentation
Documentation des usages actuels avec exemples lorsque c’est pertinent
Notes sur les évolutions possibles mais compatibles (évolutions sans BC)
Références croisées entre classes (@see, @uses)
Exemples de code concrets pour les fonctionnalités plus « complexe »
Exemple Concret
/**
* Initialise la réponse AJAX
* Configure les en-têtes HTTP et vérifie les actions autorisées en GET
*
* @param array $_allowGetAction Liste des actions autorisées en GET
* @return void
* @throws \Exception Si l'action demandée en GET n'est pas autorisée
*
* @note Évolution possible
* Cette méthode pourrait retourner $this pour permettre le chaînage :
* ```php
* ajax::init(['action'])
* ->withValidator()
* ->withFormat();
* ```
* Ce changement serait rétrocompatible
*/
public static function init($_allowGetAction = array()) {
Les objectifs sont de faciliter la compréhension du code pour les nouveaux contributeurs, maintenir une documentation à jour et utile et préparer sereinement les évolutions futures.
Cette approche vous semble-t-elle plus viable ?
Quelles ajustements ou changements à cette approche auriez-vous à proposer ?
En complément, j’ai ouvert une 2ème PR qui concerne la classe cache.class.php selon la même approche. Cela permet aussi d’avoir une vision sur une classe plus grosse.
Je reste à votre disposition pour toute discussion ou suggestion.
Merci de relancer le truc ( 3 ans + tard ) ( je suis un précurseur )
Pour reprendre ici les arguments de Salviaf, en tout cas ceux auxquel j’adhère aussi :
phpdoc en anglais, on sait jamais, si un jour Jeedom s’internationalise vraiment. En attendant ça peut motiver des non-francophone à plonger dans le code et participer…
supprimer les TODO et autres projets de roadmap (j’allais dire des plans sur la comète) car c’est plutôt des sujets à lancer sur le community (où on peut aussi discuter technique) ou dans une issue github
remplacer les exemples trop longs par des @link vers la doc developpeur de Jeedom: https://doc.jeedom.com/fr_FR/contribute/core que l’on pourra également enrichir en lien avec cette phpdoc.
concernant l’utilisation de phpdoc pour le typage (pour simplifier): on est à un tournant je pense.
Effectivement au plus on définira le type réel des arguments et les retours de fonctions, au plus ca sera automatiquement repris dans la doc après (et dans nos éditeurs favoris, ce qui pour moi reste le plus critique, je veux avoir la doc via hint box directement en ligne lorsque je dev)
mais on y est pas encore et ca va vmt être long voir dangereux de retro-fit tout le code… => je suis pour ajouter les @param et @return (et les @var lorsque nécessaire)
concernant la description d’une fonction mon point de vue est très clair (dans ma tête au moins) et j’insiste (n’en déplaise à certain):
non, aucun commentaire expliquant ce que fait une fonction ne devrait être nécessaire => si on ressent le besoin de mettre un commentaire, c’est qu’on la mal écrite et qu’on doit réécrire la fonction ou la diviser ou… ; le nom de la fonction devrait suffit à lui même pour comprendre ce qu’elle fait et donc il ne faut pas ajouter de la logique qui n’en fait pas partie, cela doit être extrait dans une autre fonction.
ca c’est la théorie, j’en conviens, du coup je ne suis pas opposé non plus à avoir une brève description et
on devrait limiter la description à une ou deux phrases maximum pour que 1/ que ca reste lisible dans les bulles d’aides dans un IDE, et 2/ cela na rallonge pas trop la taille du code
Merci @Mips pour ces retours très pertinents qui permettent d’avancer sur le sujet !
Je suis totalement d’accord avec cette vision théorique sur le nommage des méthodes. Idéalement, une méthode bien nommée et correctement découpée devrait se passer d’explication.
Cependant, en pratique, j’ai rencontré plusieurs cas dans le core (notamment dans la classe cache) où les noms des méthodes ne reflètent pas complètement leur comportement, souvent parce qu’elles ont plusieurs responsabilités. Dans ces cas-là, une description plus détaillée peut servir de « pont » :
Elle aide les développeurs à comprendre rapidement le code existant
Elle met en évidence les méthodes qui mériteraient d’être repensées
Elle facilite un futur travail de refactoring
Concernant les exemples, je rejoins l’idée de les limiter, mais je pense qu’ils gardent leur utilité au niveau de la documentation de la classe elle-même. Un exemple concis d’utilisation peut aider les développeurs de plugins à comprendre rapidement comment interagir avec la classe, surtout quand l’organisation des méthodes n’est pas optimale.
Pour les TODO, je comprends tout à fait qu’ils n’ont pas leur place dans la PHPDoc. C’était une tentative d’ouvrir la discussion sur de potentielles améliorations. Il y a sûrement de meilleurs canaux pour cela.
Et bien sûr, totalement d’accord sur l’anglais qui est la langue standard pour la documentation technique.
Je prendrais le temps de mettre à jour la classe ajax pour faire une nouvelle proposition prennant en compte ces retours.
Suite à nos échanges sur la standardisation de la PHPDoc, j’ai mis à jour la documentation de la classe ajax pour la rendre conforme aux standards discutés.
Voici les avantages et inconvénients que j’identifie pour chaque approche :
Approche 1 - @var string Maximum de simplicité Minimum de maintenance Aucune indication sur les valeurs possibles Pas d’aide à l’IDE ou aux outils d’analyse
Approche 2 - Union de littéraux Simple à mettre en place Directement lisible dans la PHPDoc générée Compatible avec tous les outils Duplication si utilisé dans plusieurs méthodes Plus difficile à maintenir si les valeurs changent
Approche 3 - phpstan-type DRY - définition unique du type Meilleure maintenabilité Plus « typé » Moins visible dans la doc générée Nécessite de consulter la définition du type
Approche 4 - Hybride Combine les avantages des approches 2 et 3 Documentation complète Plus verbeuse Risque de désynchronisation entre le type et la description
Personnellement, je pencherais pour l’approche 3 car elle me semble plus maintenable sur le long terme et plus cohérente avec une approche orientée type. De plus, elle faciliterait une éventuelle migration vers un véritable type enum PHP 8.1 dans le futur.
Qu’en pensez-vous ? Y a-t-il d’autres aspects à prendre en compte dans ce choix ?
Bon, je ne connais pas phpstan-type, c’est nativement intégré au générateur phpdoc ou c’est un plugin supplémentaire ? Est-ce qu’on est obligé de ramener la doc au début de classe, on ne peut pas faire simple ? Et aussi, pourquoi CronState alors que la variable s’appelle $state ?
Je comprends ton questionnement sur phpstan-type. Je vais essayer de clarifier certains points.
@phpstan-type est en effet une annotation spécifique à PHPStan, un outil d’analyse statique pour PHP. Ce n’est pas nativement intégré à phpDocumentor, qui est l’outil utilisé pour générer la documentation. La syntaxe que tu proposes n’est malheureusement pas valide car @phpstan-type nécessite de définir un nouveau type nommé, il ne peut pas être directement lié à une variable.
Cependant, ton idée d’avoir la documentation au plus près du code est très pertinente :
class cron {
/**
* Current cron state
* @var string 'stop'|'starting'|'run'|'stoping'|'error'
*/
protected $state = 'stop';
}
Cette approche standard est plus claire et directement utilisable avec phpDocumentor, sans nécessiter d’outils supplémentaires.
D’ailleurs, il n’existe pas de solution simple pour intégrer les annotations phpstan dans phpDocumentor, ce qui renforce l’intérêt de rester sur une syntaxe standard.
Pour info, PHPStan permet à l’IDE de détecter les erreurs de type en temps réel et de proposer l’autocomplétion sur les valeurs possibles. Par exemple, si on essaie d’assigner une valeur non autorisée à $state, l’IDE nous alertera immédiatement. Mais on peut obtenir le même résultat avec la syntaxe standard que tu proposes.
je vote pour approche 1 voir 2 si certains sont motivés
plus simple
compatibilité / standard
les autres changements sur la gestion de la doc sont déjà un « grand » pas;
je pense qu’il est préférable de ne pas introduire trop de changements à la fois pour garder tout le monde à bord, que les nouvelles habitudes s’installent petit à petit et ensuite évaluer si c’est efficace, s’il faut faire plus ou moins etc ; autrement dit, à vouloir tout, rien ne se fera.
Ok alors je comprends mieux ton mode hybride, tu essaye de donner l’info à la fois à PhpDoc et à l’IDE via PHPStan du coup, c’est redondant, je comprends que ça ne concerne que les enum puisque c’est non pris en charge par PHP avant 8.1
Il vaut mieux rester sur quelque chose de simple et standard, les IDE sont sûrement déjà compatible avec les enum PHP8.1+ et pour moi la phpdoc actuelle est déjà bien rendue dans nos IDE
Non pas du tout. L’IDE comprendra très bien le phpstan-type sans le « mode hybride ». La redondance servirait plutôt à garder les valeurs visibles au plus proche du code sans se réferrer au phpstan-type qui serait défini au début de la classe.
Je pense qu’au vu des contraintes l’approche 2 est la plus adaptée de toute façon.
( 3 ans + tard ) ( je suis un précurseur )
Aucune indication sur les valeurs possibles