[Phpmyadmin-devel] Parser and uppercase characters

[Marc Delisle]

Le 2015-06-08 15:24, Dan Ungureanu a écrit :
> On Mon, Jun 8, 2015 at 4:33 PM, Marc Delisle <marc at infomarc.info
> <mailto:marc at infomarc.info>> wrote:
> 
>     Le 2015-06-08 09:23, Dan Ungureanu a écrit :
>     > Hello Marc,
>     >
>     > `Parser::$STATEMENT_PARSERS` is a constant (its value remains the same
>     > during run-time), but I could not declare it as I should because it is
>     > illegal to declare constant arrays in versions prior to PHP 5.6. Also, I
>     > decided to avoid the `const` keyword in integer or string constants for
>     > consistency reasons. Do you think that I should rename the constant
>     > arrays and add the `const` keyword to other declarations?
> 
>     Hi Dan,
>     (please use bottom-posting on this list)
> 
>     When you mention consistency, I am concerned with consistency with the
>     phpMyAdmin codebase, which uses the 'const' keyword. Please use it,
>     except for arrays.
> 
>     >
>     > On Mon, Jun 8, 2015 at 3:14 PM, Marc Delisle <marc at infomarc.info <mailto:marc at infomarc.info>
>     > <mailto:marc at infomarc.info <mailto:marc at infomarc.info>>> wrote:
>     >
>     >     Hi Dan,
>     >
>     >     At this point, since you prefer to work in your own repository [0], I
>     >     cannot add line comments via Github so I'll send my comments to this
>     >     list.
>     >
>     >     Please explain why in [1] you are using uppercase. This should be only
>     >     for constants.
>     >
>     >
>     >     [0] https://github.com/udan11/sql-parser
>     >
>     >     [1] https://github.com/udan11/sql-parser/blob/master/src/Parser.php#L19
>     >
>     >     --
>     >     Marc Delisle | phpMyAdmin
>     >
>     >     ------------------------------------------------------------------------------
>     >     _______________________________________________
>     >     Phpmyadmin-devel mailing list
>     >     Phpmyadmin-devel at lists.sourceforge.net
>     <mailto:Phpmyadmin-devel at lists.sourceforge.net>
>     >     <mailto:Phpmyadmin-devel at lists.sourceforge.net
>     <mailto:Phpmyadmin-devel at lists.sourceforge.net>>
>     >     https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>     >
>     >
>     >
>     >
>     >
>     ------------------------------------------------------------------------------
>     >
>     >
>     >
>     > _______________________________________________
>     > Phpmyadmin-devel mailing list
>     > Phpmyadmin-devel at lists.sourceforge.net
>     <mailto:Phpmyadmin-devel at lists.sourceforge.net>
>     > https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>     >
> 
> 
>     --
>     Marc Delisle | phpMyAdmin
> 
>     ------------------------------------------------------------------------------
>     _______________________________________________
>     Phpmyadmin-devel mailing list
>     Phpmyadmin-devel at lists.sourceforge.net
>     <mailto:Phpmyadmin-devel at lists.sourceforge.net>
>     https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
> 
> 
>  Hello Marc,
> 
> I changed declarations for constants.
> 
> Regarding the implementation of `sql-parser` in phpMyAdmin. I believe
> that the place of the library should be in the `libraries` directory
> among other libraries, but I am not sure if the tests should be in there
> as well. I see that no other library includes tests, but that might be
> just because they have no tests. Including tests would increase the size
> of the phpMyAdmin package with about 200kb (uncompressed) and I know
> that the team tries to keep the size of the package as small as possible.

Have you looked under test/libraries?

Before generating the downloadable packages, we remove the test
directory, so it has no impact on the size.

> 
> The other question I have is about autoloading classes. For this library
> I used the autoloader that is generated by Composer. However, phpMyAdmin
> does not use Composer for managing dependencies and showed no interest
> in starting using it (I proposed it in the past), so now I have to find
> a way to include the required files. Possible solutions: a) manually
> include each class in each file; b) include all files in the beginning;
> c) use the generated Composer autoloader; d) write another smaller
> autoloader. I think that c) is the best solution, but I would love to
> hear your opinions as well.

Looks like c) is the best, but I'm not familiar enough with Composer to
be sure, so I'll welcome other opinions.


-- 
Marc Delisle | phpMyAdmin