<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jun 8, 2015 at 4:33 PM, Marc Delisle <span dir="ltr"><<a href="mailto:marc@infomarc.info" target="_blank">marc@infomarc.info</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>Le 2015-06-08 09:23, Dan Ungureanu a écrit :<br>
> Hello Marc,<br>
><br>
> `Parser::$STATEMENT_PARSERS` is a constant (its value remains the same<br>
> during run-time), but I could not declare it as I should because it is<br>
> illegal to declare constant arrays in versions prior to PHP 5.6. Also, I<br>
> decided to avoid the `const` keyword in integer or string constants for<br>
> consistency reasons. Do you think that I should rename the constant<br>
> arrays and add the `const` keyword to other declarations?<br>
<br>
</span>Hi Dan,<br>
(please use bottom-posting on this list)<br>
<br>
When you mention consistency, I am concerned with consistency with the<br>
phpMyAdmin codebase, which uses the 'const' keyword. Please use it,<br>
except for arrays.<br>
<span><br>
><br>
> On Mon, Jun 8, 2015 at 3:14 PM, Marc Delisle <<a href="mailto:marc@infomarc.info" target="_blank">marc@infomarc.info</a><br>
</span><span>> <mailto:<a href="mailto:marc@infomarc.info" target="_blank">marc@infomarc.info</a>>> wrote:<br>
><br>
> Hi Dan,<br>
><br>
> At this point, since you prefer to work in your own repository [0], I<br>
> cannot add line comments via Github so I'll send my comments to this<br>
> list.<br>
><br>
> Please explain why in [1] you are using uppercase. This should be only<br>
> for constants.<br>
><br>
><br>
> [0] <a href="https://github.com/udan11/sql-parser" target="_blank">https://github.com/udan11/sql-parser</a><br>
><br>
> [1] <a href="https://github.com/udan11/sql-parser/blob/master/src/Parser.php#L19" target="_blank">https://github.com/udan11/sql-parser/blob/master/src/Parser.php#L19</a><br>
><br>
> --<br>
> Marc Delisle | phpMyAdmin<br>
><br>
> ------------------------------------------------------------------------------<br>
> _______________________________________________<br>
> Phpmyadmin-devel mailing list<br>
> <a href="mailto:Phpmyadmin-devel@lists.sourceforge.net" target="_blank">Phpmyadmin-devel@lists.sourceforge.net</a><br>
</span>> <mailto:<a href="mailto:Phpmyadmin-devel@lists.sourceforge.net" target="_blank">Phpmyadmin-devel@lists.sourceforge.net</a>><br>
> <a href="https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel" target="_blank">https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel</a><br>
<div><div>><br>
><br>
><br>
><br>
> ------------------------------------------------------------------------------<br>
><br>
><br>
><br>
> _______________________________________________<br>
> Phpmyadmin-devel mailing list<br>
> <a href="mailto:Phpmyadmin-devel@lists.sourceforge.net" target="_blank">Phpmyadmin-devel@lists.sourceforge.net</a><br>
> <a href="https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel" target="_blank">https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel</a><br>
><br>
<br>
<br>
--<br>
Marc Delisle | phpMyAdmin<br>
<br>
------------------------------------------------------------------------------<br>
_______________________________________________<br>
Phpmyadmin-devel mailing list<br>
<a href="mailto:Phpmyadmin-devel@lists.sourceforge.net" target="_blank">Phpmyadmin-devel@lists.sourceforge.net</a><br>
<a href="https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel" target="_blank">https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel</a></div></div></blockquote><div><br></div><div> Hello Marc,</div><div><br></div><div>I changed declarations for constants.</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>Thank you!</div></div></div></div>