[phpMyAdmin Developers] Question about PSR1

[Marc Delisle]

Le 2015-12-05 10:21, Richard Damon a écrit :
> On 12/5/15 9:37 AM, Marc Delisle wrote:
>> Le 2015-12-05 09:08, Hugues Peccatte a écrit :
>>>
>>> Le sam. 5 déc. 2015 à 13:57, Marc Delisle <marc at infomarc.info
>>> <mailto:marc at infomarc.info>> a écrit :
>>>
>>>      Hi,
>>>
>>>      We have some PSR1 Scrutinizer warnings in many files. For
>>> example, in
>>>      js/whitelist.php:
>>>
>>>      "For compatibility and reusability of your code, PSR1 recommends
>>> that a
>>>      file should either new symbols (like classes, functions, etc.)
>>> or have
>>>      side-effects (like outputting something, or including other
>>> files), but
>>>      not both at the same time. The first symbol is defined on line
>>> 20 and
>>>      the first side effect is on line 9."
>>>
>>>      Is it true that PSR1 objects to our having a define() statement and
>>>      ordinary code like chdir() in the same file?
>>>
>>>      --
>>>      Marc Delisle | phpMyAdmin
>>>
>>>      _______________________________________________
>>>      Developers mailing list
>>>      Developers at phpmyadmin.net <mailto:Developers at phpmyadmin.net>
>>>      https://lists.phpmyadmin.net/mailman/listinfo/developers
>>>
>>> Hi,
>>>
>>> Yes, that's true because one of the rules is not to have declaration and
>>> logic code in the same file. Define is declaration, chdir is logic.
>>>
>>> H.
>> Do you suggest we ignore this rule? To respect it, we would need to
>> change our constants to something else.
>>
>>
> PSR1 lists this as a 'SHOULD', not a 'MUST'.
> 
> The purpose of this rule, as I understand it, is that 'reusable' stuff
> should be reusable by including the file that define it, and shouldn't
> force some action to be performed. Also, that includes that 'do
> something' may be needed in multiple places, and if they define stuff,
> they can't be multiply included.
> 
> If the define()s are purely for internal usage, and the file has no
> possible reason for wanting to be included twice, then we may fall
> within a valid exception to PSR-1.
> 
> To be strictly meeting it, the definitions (define() etc) should be
> factored out into an include file that is required_once.
> 

I find it overkill to move one line of define() to an include file. A
possible way to comply with PSR1 in this situation would be the following.

In js/whitelist.php, replace the define() with
$pmaMinimumCommon = true;

In libraries/common.inc.php replace the if (! defined()) with
if (! pmaMinimumCommon)

-- 
Marc Delisle | phpMyAdmin