[Beta] LMDI Multilinks 2.0.0

Pour les extensions destinées à phpBB 3.1.x

Modérateur: Equipe

Règles du forum
A lire impérativement : Règlement de phpBB-fr.com

Re: LMDI Multilinks 2.0.0

Messagepar Skouat » 15 Avr 2017 à 12:17

Hello,

Dans le fichier de migration tu n'as pas besoin de déclarer la globale $table_prefix.
Elle est déjà déclarée et est utilisable via $this->table_prefix

Donc
Code: Tout sélectionner
	public function revert_schema()
	{
		global $table_prefix;
		$table = $table_prefix . 'lmdi_multilinks';
			return array(
				'drop_tables'   => array(
					$table,
				)
			);
	}

devient
Code: Tout sélectionner
    public function revert_schema()
    {
            return array(
                'drop_tables'   => array(
                    $this->table_prefix . 'lmdi_multilinks',
                )
            );
    


corrections à effectuer sur la méthode update_schema()




Code: Tout sélectionner
						'ppap'		=> array('BOOL', 0),	// 0 = prepend, 1 = append
						'enabled'		=> array('BOOL', 0),
						'blank'		=> array('BOOL', 0),	// target = _blank
						'usicon'		=> array('BOOL', 1),	// Icon by default
						'usfile'		=> array('BOOL', 0),	// Icon by default

ce que t'as écrit est correct, mais par convention bool => "true/false" et non "0/1"




Code: Tout sélectionner
	static public function depends_on()
	{
		return array('\phpbb\db\migration\data\v310\alpha2');
	}

La c'est trivial et ne change rien au fonctionnement de ton extension, mais pourquoi ne pas dépendre de la version stable de 3.1.0 ?




Dans listener.php

Code: Tout sélectionner
define ("_PP_" , 0);
define ("_AP_" , 1);

Sauf erreur de ma part, les constantes doivent être déclarées dans une classe




private function assign_block_vars($ppap, $block)
Je ne pense pas qu'il soit judicieux d'utiliser le même nom de méthode que celle de phpBB.




Code: Tout sélectionner
		$sql = "SELECT * FROM " . $this->table . " WHERE ppap = $ppap AND enabled = 1";

1/ il faut préférer les ' au "
2/ Tu dois échapper la variable $ppap

Donc à remplacer par
Code: Tout sélectionner
$sql = 'SELECT * FROM ' . $this->table . ' WHERE ppap = ' . (bool) $ppap . ' AND enabled = 1'


Après je ne vois pas trop l'intérêt des constantes si tu ne dois gérer qu'un true/false.




$usfile = $row['usfile'];
Variable déclarée, mais non utilisée. Donc à supprimer ou à utiliser.




Dans multilinks_module.php
Code: Tout sélectionner
		$enabled = (int) $request->variable('ml_enabled', false);
		$blank = (int) $request->variable('ml_blank', false);
		$usicon = (int) $request->variable('use_icon', false);
		$usfile = (int) $request->variable('use_file', false);

n'est-ce pas un peu incohérent ?
Tu testes du booleen que tu cast en entier. :shock:




Code: Tout sélectionner
$item_id = $db->sql_nextid();

A quoi bon assigner le nextid à $item_id si tu n'exploite pas $item_id ?


Voila, c'était un relecture ultra rapide de ton code.

A+
Mes MODS

Ultima-World Hébergé par phpBB-Services
Code parrainage : 1241646554
Skouat
Traducteur
Traducteur
 
Messages: 13031
Enregistré le: 02 Avr 2008 à 20:47

Re: LMDI Multilinks 2.0.0

Messagepar pierredu » 15 Avr 2017 à 15:58

Merci pour tes remarques.
Les retypages en entier, c'est pour ne pas avoir d'erreur dans les instructions d'insertion (le true passe comme 1, mais le false non).
Avatar de l’utilisateur
pierredu
Extensions
Extensions
 
Messages: 862
Enregistré le: 29 Mai 2011 à 06:49
Localisation: Paris

Précédente

Retourner vers Extensions en développement

 


  • Articles en relation
    Réponses
    Vues
    Dernier message

Qui est en ligne

Utilisateurs parcourant ce forum : Aucun utilisateur enregistré et 1 invité