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@infomarc.info <mailto:marc@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@phpmyadmin.net <mailto:Developers@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. -- Richard Damon

Le sam. 5 déc. 2015 à 17:12, Marc Delisle <marc@infomarc.info> a écrit :

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@infomarc.info <mailto:marc@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@phpmyadmin.net <mailto:Developers@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

Hi, That might be the solution. I agree that constant definition might be considered as logic code. H.

Le 2015-12-07 10:46, Michal Čihař a écrit :

Hi

Dne 5.12.2015 v 16:51 Marc Delisle napsal(a):

...

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)

Maybe it would be better (and cleaner) to split up common.inc.php to two files - one doing the "minimal" set of things, the other one adding other logic on top of it.

Good point! -- Marc Delisle | phpMyAdmin