modification de l''override - Grand mécontentement

[chantane]

Bonjour,

 

Il semble que la classe ControllerFactory va disparaitre et que tous les controllers seront surchargés par défaut dans le dossier override, même s'ils sont quasiment vides. Pouvez vous me le confirmer ?

 

Auquel cas, c'est une bonne idée qui disparait et une déception !

 

Un intérêt de l'override pour un développeur est de repérer au premier coup d’œil les fichiers et fonctions à mettre à jour pour chaque release.

Le dossier override de la 1.4 ne contenait que les quelques fichiers modifiés, ceci permettait de ne pas faire de vérification fichier par fichier pour savoir s'il était modifié et de nous simplifier la vie lors d'intervention sur les sites en cas de problème.

 

Cette nouvelle version nous amène bien des améliorations, mais sur ce point là, lors des maj ou dépannage, vous nous ramenez en arrière ! Le dossier override ne devrait contenir que les fichiers surchargés !

Merci.

[Raphaël Malié]

Bonjour chantane,

la classe ControllerFactory ne servait à rien, et n'était qu'une réplique de l'autoloader c'est pour cela qu'elle a été supprimée.

Ce que vous pointez est effectivement le seul défaut à la présence des fichiers dans l'override que je vois, cela dit il y a plusieurs avantages :

1. Les développeurs pourront plus facilement voir comment l'override fonctionne, et ainsi l'utiliser (j'ai vu de très / trop nombreuses personnes au barcamp qui ne connaissaient malheureusement pas ce système et continuaient à faire leurs propres modifications dans les fichiers cœurs !)
   
2. Maintenant les IDE peuvent enfin faire de l'autocomplétion sur les classes, ce qui est clairement pratique pour un développeur
   

Vous pouvez largement palier au soucis en faisant un script qui vous listera les fichiers modifiés non ?

 

Cordialement

[chantane]

Merci Raphaël,

- pour le point 1, On trouve maintenant d'avantage d'exemples sur l'override de PS, et des guides très clair sur ce sujet.

donc pour ceux qui connaisse mal la programmation objet et PS, je ne suis pas certaine que la présence de ces fichiers les aide davantage.

 

- pour le point 2, l'autocomplétion pourquoi ne pas déclarer ces classes vides dans un seul fichier, à part, qui ne serait utile que pour cela.

Il y a moyen de repondre à l'autocomplétion dans les IDE sans utiliser le dossier override qui serait et devrait être destiné aux modifications.

 

Vous pouvez largement palier au soucis en faisant un script qui vous listera les fichiers modifiés non ?

Est-ce une plaisanterie ?

 

La hiérarchisation des dossiers est un point important pour s'y retrouver, surtout lorsque l'on travaille sur plus d'un site.

Avoir un dossier réservé aux fichiers personnalisés et uniquement est un plus pour le développeur.

Il est vraiment dommage que vous ne proposiez comme seule alternative que de développer un script spécifique pour y retrouver ses "petits". C'est une solution plutôt bancale.

[Raphaël Malié]

Je suis entièrement d'accord pour le point 1, ceux qui ont des mauvaises pratiques ne les abandonneront pas de sitôt, cela dit il faut les limiter au maximum. Cependant l'argument est autant valable que le votre que la liste des fichiers modifiés, honnêtement il suffit de voir ceux qui font plus que X octets pour voir qu'on les a modifié

 

Une autre raison qui a fait qu'on a mis ces fichiers override est qu'avant ça on faisait un horrible eval(), et voir des eval() ça fait tilter

 

Pour moi l'important c'est de se concentrer sur un outil qui détecte les modifications, à priori on souhaite intégrer un fichier contenant les md5 des fichiers depuis longtemps, et j'aimerai qu'on puisse le faire pour la RC, ça répondrait au problème.

[olea]

Une autre raison qui a fait qu'on a mis ces fichiers override est qu'avant ça on faisait un horrible eval(), et voir des eval() ça fait tilter

Ca me semble être la véritable raison

 

Les autres raisons se valent. Pour info, sous Eclipse, pas facile de voir la taille des fichiers au premier coup d'oeil

Par contre, avoir l'autocomplétion sera très pratique

 

Effectivement, chacun adaptera son environnement pour détecter plus facilement les modifications

[olea]

Raphael,

 

Dans la page Back-office donnant l'environnement, utile lors de debug (version PHP, apache...), vous pourriez également donner la liste des fichiers override que vous estimez être modifiés.

Plutôt que chacun fasse son script, autant l'embarquer dans cette page

[chantane]

un outil du BO pour retrouver les fichiers modifiés ferait bien l'affaire.

Merci,

[chris13]

Bonjour,

 

Je confirme que le répertoire override devrait contenir uniquement les fichiers des classes surchargées. Ce n'est pas très agréable d'avoir la liste de tous les fichiers dans ce répertoire pour des raisons de lisibilité, d'accessibilité, et d'administration.

 

Si c'est uniquement pour répondre aux avantages 1/ et 2/ énoncés ci-dessus, je trouve que votre solution est un peu démesurée.

 

L'avantage de centraliser les modifications et de pouvoir y accéder rapidement est bien plus important pour un administrateur/développeur que l'autocomplétion (qu'il pourrait gérer par un autre moyen).

 

J'espère vraiment que vous allez revenir au fonctionnement de la 1.4 en ce qui concerne le répertoire override, et non pas ajouter une page au BO avec la liste des fichiers modifiés.

[chantane]

Bonjour,

Je n'ai pas testé mais en regardant le code, les classes non surchargées peuvent être instanciées par un eval (couteux) comme dans la 1.4. Donc il doit être possible de simplement supprimer les fichiers "vides" du dossier override et d'avoir ainsi une vision plus claire des fichiers modifiés.

[Broceliande]

Bonjour,

Je n'ai pas testé mais en regardant le code, les classes non surchargées peuvent être instanciées par un eval (couteux) comme dans la 1.4. Donc il doit être possible de simplement supprimer les fichiers "vides" du dossier override et d'avoir ainsi une vision plus claire des fichiers modifiés.

 

Hmm j'émet un gros doute là dessus .

S'il est nécessaire de posséder tous les fichiers dans le dossier override afin d'éviter des eval() tant et à juste titre décriés , je vois mal comment la méthode remplaçante pourrait se dispenser de ces fichiers , même vides , encore que je ne me sois pas encore penché sur la question concernant la 1.5

[Raphaël Malié]

Sisi je confirme que le eval() est bel et bien utilisé dans le cas où la classe est manquante, pour des raisons de rétrocompatibilité.

[Broceliande]

Ah j'ajoute un gros +1 à la remarque d'Olea , à savoir que ce n'est pas un luxe d'avoir l'auto-complétion dans nos IDEs préférés . Or l'autoload de la 1.4 rend cette auto-complétion bien plus ardue (dans netbeans par exemple ça fonctionne , mais à force d'usage et au petit bonheur la chance)

[Broceliande]

Sisi je confirme que le eval() est bel et bien utilisé dans le cas où la classe est manquante, pour des raisons de rétrocompatibilité.

 

Oui mais si et seulement si ... ce qui est un moindre mal.

 

Edit : encore que pour le coup celà ne donne toujours aucun intérêt à supprimer les overrides natives de la 1.5

[chris13]

Edit : encore que pour le coup celà ne donne toujours aucun intérêt à supprimer les overrides natives de la 1.5

 

Vous ne voyez pas l'intérêt d'avoir un dossier override contenant uniquement les classes réellement surchargées ?

[Broceliande]

 

Vous ne voyez pas l'intérêt d'avoir un dossier override contenant uniquement les classes réellement surchargées ?

 

Evidemment que si , mais à mon sens c'est juste une question de lisibilité, sauf que dans le cas présent c'est une question de méthode et de choix du moindre mal, si je me réfère à la réponse de Raphaël.

 

En gros je comprends que l'alternative permettant d'éviter l'utilisation d' eval() implique d'avoir au minimum une classe de surcharge vide pour chaque classe existante.

Si tel est le cas , qu'à cela ne tienne , on se place dans un schéma "put your own code here" qui nous évitera peut être de passer des heures à réparer ou mettre à jour un site entièrement bidouillé par un autre développeur dans les classes natives.

A nouveau, avoir l'auto-complétion entièrement fonctionnelle dans notre IDE favori est aussi un gain de temps non négligeable.

Et enfin on évite l'utilisation d' eval dans l'autoload.

 

Au final, l'idée d'avoir cette liste de fichiers dans le dossier override me semble un mal pour un bien.

 

Ce qui peut manquer simplement serait probablement un xml contenant le checksum de chacun de ces fichier dans leur version "vide" et un adminTab permettant de lister facilement les fichiers modifiés.

Ce n'est pas un gros deal, la team y a peut être déja pensé...

[Raphaël Malié]

Le débat a l'air de passionner. Honnêtement je défend cette cause car j'ai été un des devs à la vouloir et à réécrire l'autoload, cela dit il est important que tous les devs travaillant sur PrestaShop (que ce soit nous ou la communauté) soient content. Au final l'ajout des MD5 dans les release devrait contenter tout le monde, puisqu'il sera possible de voir directement les fichiers modifiés que ce soit dans l'override ou ailleurs dans le code.

[chris13]

Au final l'ajout des MD5 dans les release devrait contenter tout le monde, puisqu'il sera possible de voir directement les fichiers modifiés que ce soit dans l'override ou ailleurs dans le code.

 

Vous allez donc laisser 1 fichier de surcharge par classe dans le dossier override, mais il sera possible de voir les fichiers modifiés via une interface dans le backoffice ?

[Raphaël Malié]

Ben c'est ce que je souhaite en tous cas, et je ferai au max pour l'ajouter à la 1.5

[chantane]

Je suis plutôt d'accord avec cette solution.

Avoir une table de comparaison des md5 des versions courante et précédente serait un plus.

Ceci permettrait de connaitre les classes ou controllers modifiés entre deux versions, et pas seulement entre la version courante et nos surcharges.

Merci

[TrogloGeek]

Ne pourrait on pas avoir deux dossiers distincts et une règle de priorité dans l'autoload comme sous Magento (app/code/core; app/code/community; app/code/local) ?

 

Dans notre cas on aurait par exemple un dossier natives​ ou core parallèle à override dans sa structure contenant toutes les coquilles de surcharges vides, et si le fichier est présent dans override celui de natives serait ignoré.

 

On aurait à la fois la simplicité des mises à jour et un héritage pris en compte dans les EDI, cela au coût d'un file_exists pour chaque classe instanciée, ou en configurant l'include_path pour que cette gestion de priorité soit automatique, encore une fois comme sous Magento.

 

On peut même prévoir en ce cas un système de cache/compilation (déclenchable dans le back office) qui va chercher les fichiers en respectant la règle de priorité et les copie dans le répertoire de cache/compilation, auquel cas l’auto-loader appellerait directement le fichier dans ce dossier sans avoir à effectuer de test. Le désavantage minime étant de devoir relancer la compilation à chaque ajout/retrait/modification d'un override. A des fins de développement/débogage, le cache devrait pouvoir être désactivé.

 

Concernant l’auto-complétion 1.4 sous Eclipse (devrait fonctionner sous d'autres EDI), il suffit d'un peu d'astuce :

http://prestashop.blog.capillotracteur.fr/2012/02/17/prestashop-1-4-x-et-eclipse-pdt/

[TrogloGeek]

Au final l'ajout des MD5 dans les release devrait contenter tout le monde, puisqu'il sera possible de voir directement les fichiers modifiés que ce soit dans l'override ou ailleurs dans le code.

Attention, en ce cas le md5 devrait idéalement être généré après normalisation des retours à la ligne, sans quoi un fichier qui serait passé par un FTP en mode ASCII pourrait apparaître comme modifié. (et les boulets transfèrent quasiment tous leurs fichiers PHP par FTP en mode ASCII ;-) )

[coeos.pro]

Bonjour à tous, il est vrai qu'avant il suffisait de vérifier l'existence d'un fichier pour savoir si on pouvait lors de l’installation copier un fichier d'un module directement vers le dossier override. En fait avec la version c'est un "problème" mais un faux problème, suffit d'utiliser par exemple une fonction du genre :

 

function get_overriden_methods($class, $method)
[indent=1]
{
if(!get_class_methods($class))
[indent=1]return true;[/indent]

   $rClass = new ReflectionClass($class);

   foreach ($rClass->getMethods() as $rMethod)
[indent=1]    {[/indent]
[indent=1]        try[/indent]
[indent=1]        {[/indent]
[indent=1]            // attempt to find method in parent class[/indent]
[indent=1]            new ReflectionMethod($rClass->getParentClass()->getName(), $rMethod->getName());[/indent]
[indent=1]            // check whether method is explicitly defined in this class[/indent]
[indent=1]            if ($rMethod->getDeclaringClass()->getName() == $rClass->getName() AND ($rMethod->getName() == $method))[/indent]
[indent=1]return False;[/indent]

[indent=1]        }[/indent]
[indent=1]        catch (exception $e)[/indent]
[indent=1]        {    /* was not in parent class! */    }[/indent]
[indent=1]    }    [/indent]
   return true;
}[/indent]



public function installation_de_mon_module()
[indent=1]{[/indent]
[indent=1]...[/indent]

[indent=1]$override_possible = $this->get_overriden_methods('AuthController', 'updatdeContext');[/indent]
[indent=1]if($override_possible)[/indent]
[indent=2]{...}[/indent]
[indent=1]else[/indent]
[indent=2]{...}[/indent]
[indent=1]...[/indent]

 

code testé,

$override_possible = true : on peut y aller, la fonction n'est pas dans l'override

$override_possible = false : zut, la fonction est déjà overridée, va falloir y aller à la mano

[coeos.pro]

<p>zut il y a des

qui gâchent tout, et PAS DE BOUTONS MODIFIER ???</p>

<p> </p>

<p>sans indentation : </p>

<p> </p>

<p> </p>

<p> </p>

<div>function get_overriden_methods($class, $method)</div>

<div> </div>

<div>{</div>

<div>if(!get_class_methods($class))</div>

<div>return true;</div>

<div> </div>

<div>$rClass = new ReflectionClass($class);</div>

<div> </div>

<div>foreach ($rClass->getMethods() as $rMethod)</div>

<div>{</div>

<div>try</div>

<div>{</div>

<div>// attempt to find method in parent class</div>

<div>new ReflectionMethod($rClass->getParentClass()->getName(), $rMethod->getName());</div>

<div>// check whether method is explicitly defined in this class</div>

<div>if ($rMethod->getDeclaringClass()->getName() == $rClass->getName() AND ($rMethod->getName() == $method))</div>

<div>return False;</div>

<div> </div>

<div>}</div>

<div>catch (exception $e)</div>

<div>{    /* was not in parent class! */    }</div>

<div>}    </div>

<div>return true;</div>

<div>}</div>

<div> </div>

<div> </div>

<div> </div>

<div>public function installation_de_mon_module()</div>

<div>{</div>

<div>...</div>

<div> </div>

<div>$override_possible = $this->get_overriden_methods('AuthController', 'updatdeContext');</div>

<div>if($override_possible)</div>

<div>{...}</div>

<div>else</div>

<div>{...}</div>

<div>...</div>

 

[coeos.pro]

décidemment...

j'espère ce sera bon cette fois ci

 

function get_overriden_methods($class, $method)
{
if(!get_class_methods($class))
	return true;

$rClass = new ReflectionClass($class);

foreach ($rClass->getMethods() as $rMethod)
	{
	try
		{
		// attempt to find method in parent class
		new ReflectionMethod($rClass->getParentClass()->getName(), $rMethod->getName());
		// check whether method is explicitly defined in this class
		if ($rMethod->getDeclaringClass()->getName() == $rClass->getName() AND ($rMethod->getName() == $method))
			return False;
		}
	catch (exception $e)
		{    /* was not in parent class! */    }
	}    
return true;
}



public function installation_de_mon_module()
{
...
$override_possible = $this->get_overriden_methods('AuthController', 'updatdeContext');
if($override_possible)
	{...}
else
	{...}
...

 

 

$override_possible = true : on peut y aller, la fonction (ici updatdeContext) n'est pas dans l'override (ici AuthController)

$override_possible = false : zut, la fonction est déjà overridée, va falloir y aller à la mano