New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabled lint for console. #1799

Merged
merged 1 commit into from Jul 19, 2015

Conversation

Projects
None yet
3 participants
@udan11
Member

udan11 commented Jul 19, 2015

Enabled lint for console.
Updated sql-parser library to udan11/sql-parser@5e554b3.

Signed-off-by: Dan Ungureanu udan1107@gmail.com

Enabled lint for console.
Updated sql-parser library to udan11/sql-parser@5e554b3.

Signed-off-by: Dan Ungureanu <udan1107@gmail.com>
@Tithugues

This comment has been minimized.

Contributor

Tithugues commented Jul 19, 2015

Hi,
Would it be possible to get your sql-parser from composer, instead of including it in pMA sources ?
Thanks.

@lem9 lem9 self-assigned this Jul 19, 2015

@udan11

This comment has been minimized.

Member

udan11 commented Jul 19, 2015

@Tithugues I proposed using Composer in the past. There's been a discussion on the mailing list where Marc raised a few questions. I can't link it to you now because Sourceforge is in maintenance mode, but as soon as it's up you can look for "Outdated libraries" (or you can check you mail archive if you didn't delete it by now), that was the subject of the discussion.

@lem9

This comment has been minimized.

Contributor

lem9 commented Jul 19, 2015

@udan11 Apart from the "Outdated libraries" discussion, I believe that you and I had a discussion specifically about this library. I can't seem to find it right now, but I remember that the outcome was to not use Composer for this library. Any insight you could give about this other discussion?

@lem9

This comment has been minimized.

Contributor

lem9 commented Jul 19, 2015

Nice work.

lem9 added a commit that referenced this pull request Jul 19, 2015

Merge pull request #1799 from udan11/lint_console
Enabled lint for console.

@lem9 lem9 merged commit 7fbb82a into phpmyadmin:master Jul 19, 2015

0 of 2 checks passed

Scrutinizer Scheduled/Installing
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@udan11

This comment has been minimized.

Member

udan11 commented Jul 19, 2015

From "[Phpmyadmin-devel] Parser and uppercase characters". The quoted text is my question.

> 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.
@lem9

This comment has been minimized.

Contributor

lem9 commented Jul 20, 2015

Given that the library's error messages will be localized using phpMyAdmin's gettext mechanism, is it still relevant to include it via Composer instead of being directly in the repo?

@udan11 udan11 deleted the udan11:lint_console branch Jul 20, 2015

@udan11

This comment has been minimized.

Member

udan11 commented Jul 21, 2015

I did my best to make the library independent of any other component, but making it independent of phpMyAdmin's gettext mechanism would require implementing a new system for handling translations and it's not worth it and would also complicate the translation process a lot.

I still think that the best method to manage the libraries is via Composer, even though they are localized by phpMyAdmin.

@lem9

This comment has been minimized.

Contributor

lem9 commented Jul 21, 2015

About composer, we should really talk it more, but I'm not sure that phpmyadmin-devel is back online.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment