Add a "vendor" top level namespace #126

Closed
remicollet opened this Issue Jan 21, 2017 · 10 comments

Projects

None yet

4 participants

@remicollet
Contributor
remicollet commented Jan 21, 2017 edited

From http://www.php-fig.org/psr/psr-4/

The fully qualified class name MUST have a top-level namespace name, also known as a “vendor namespace”.

Ok, you can say "SqlParser" is the vendor... a way to circumvent PSR-4 (as the project SubNamespaceName is not mandatory.. one reason why I dislike this PSR). And SqlParser could be a bit too generic and may create conflict.

Proposal: use PhpMyAdmin\SqlParser

I understand this is only doable as part of a new major version, because of BC break.

@nijel
Member
nijel commented Jan 21, 2017

This pretty much applies to all libraries we ship, on the other side doing that before phpMyAdmin 4.7.0 is release would save us some headaches with possible rename in future.

Anyway this really needs feedback from other @phpmyadmin/developers ...

@nijel nijel added the enhancement label Jan 21, 2017
@ibennetch
Contributor

I don't have enough of an opinion to state a decision one way or the other.

I don't see any problem with doing this; even for other programs that use the library it shouldn't be a problem to reference the PhpMyAdmin namespace. On the other hand, I don't see why one would have two SQL parsing libraries with the same "SqlParser" namespace in one project, so I imagine the possibility of collision is quite low.

I'll go along with whatever Michal and the others decide, but have no reason to object to such a change.

@remicollet
Contributor

This pretty much applies to all libraries we ship

Indeed.

Anyway this really needs feedback

Yes, I open this issue mostly to raise such discussion. No urgency.

@nijel
Member
nijel commented Jan 21, 2017

Well there is sort of urgency - 4.7.0 beta will be out probably next week and such change should be done prior to it (or wait for 4.8.0, what will postpone it for year or so).

@bigfoot90
Contributor
bigfoot90 commented Jan 21, 2017 edited

I think you should follow semantic versioning, and don't introduce BC break until 5.0.
I'm not opposing to this change, but not in 4.*, please create a new branch for next-version.

@nijel
Member
nijel commented Jan 21, 2017

This is about phpMyAdmin versions, not the library versions. Of course this would need major version increase on the library side.

@bigfoot90
Contributor

You're right.

@nijel nijel added a commit to phpmyadmin/motranslator that referenced this issue Jan 23, 2017
@nijel nijel Add PhpMyAdmin prefix to namespace
This way we follow PSR-4.

See phpmyadmin/sql-parser#126

Signed-off-by: Michal Čihař <michal@cihar.com>
dbb5235
@nijel nijel self-assigned this Jan 23, 2017
@nijel
Member
nijel commented Jan 23, 2017

In the end I think this is sane thing to do and should be done now - before phpMyAdmin 4.7.0 first beta is released. I've started to adjust the libraries right now.

@nijel nijel added a commit that referenced this issue Jan 23, 2017
@nijel nijel Update to use motranslator 3.0
It uses PSR-4 prefix for PhpMyAdmin namespace.

See #126

Signed-off-by: Michal Čihař <michal@cihar.com>
ac852c7
@nijel nijel added a commit to phpmyadmin/phpmyadmin that referenced this issue Jan 23, 2017
@nijel nijel Update to use motranslator 3.0
It uses PSR-4 prefix for PhpMyAdmin namespace.

See phpmyadmin/sql-parser#126

Signed-off-by: Michal Čihař <michal@cihar.com>
15909c3
@nijel nijel added a commit to phpmyadmin/shapefile that referenced this issue Jan 23, 2017
@nijel nijel Add PhpMyAdmin prefix to namespace
This way we follow PSR-4.

See phpmyadmin/sql-parser#126

Signed-off-by: Michal Čihař <michal@cihar.com>
5289c4e
@nijel nijel added a commit to phpmyadmin/phpmyadmin that referenced this issue Jan 23, 2017
@nijel nijel Update to use shapefile 2.0
It uses PSR-4 prefix for PhpMyAdmin namespace.

See phpmyadmin/sql-parser#126

Signed-off-by: Michal Čihař <michal@cihar.com>
b87362b
@nijel nijel added a commit that closed this issue Jan 23, 2017
@nijel nijel Added PhpMyAdmin namespace prefix to follow PSR-4.
Fixes #126

Signed-off-by: Michal Čihař <michal@cihar.com>
428edcc
@nijel nijel closed this in 428edcc Jan 23, 2017
@remicollet
Contributor

Great thanks for the work on SqlParser/MoTranslator/ShapFile, this will make things much more clean.

@nijel nijel added a commit to phpmyadmin/phpmyadmin that referenced this issue Jan 23, 2017
@nijel nijel Update to SQL Parser 4.0.1
It now follows PSR-4 and uses PhpMyAdmin namespace prefix.

See phpmyadmin/sql-parser#126

Signed-off-by: Michal Čihař <michal@cihar.com>
e4bf536
@nijel
Member
nijel commented Jan 23, 2017

Okay, I think we're done. The change was done in phpMyAdmin master, the QA_4_6 branch stayed on the old version, in case new version would be needed for whatever reasons, there is phpmyadmin/phpmyadmin@e4bf536 which applies on QA_4_6, but needs deeper testing before it can be pushed to our users.

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