[Phpmyadmin-devel] Parser and uppercase characters

[Dan Ungureanu]

On Mon, Jun 8, 2015 at 4:33 PM, Marc Delisle <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>> 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>
> >     https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
> >
> >
> >
> >
> >
> ------------------------------------------------------------------------------
> >
> >
> >
> > _______________________________________________
> > Phpmyadmin-devel mailing list
> > 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
> 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.

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.

Thank you!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20150608/c051a355/attachment.html>