Skip to content

[4] Rename and namespace Joomla Commands#31941

Closed
PhilETaylor wants to merge 1 commit intojoomla:4.0-devfrom
PhilETaylor:namepace-commands
Closed

[4] Rename and namespace Joomla Commands#31941
PhilETaylor wants to merge 1 commit intojoomla:4.0-devfrom
PhilETaylor:namepace-commands

Conversation

@PhilETaylor
Copy link
Contributor

@PhilETaylor PhilETaylor commented Jan 9, 2021

A decision is needed asap - as a release blocker - because if this is not merged before Joomla 4.0.0 is released, then its a b/c change and will not be worth doing in the future when Joomla 4.1 is released.

Summary of Changes

prefix all Joomla internal commands with joomla: to correctly namespace them from other commands.

Out of the box this doesn't seem to make much sense, however as the community add more and more commands, the name spacing and naming convention of commands can, and will, get messy. This has already played out in other projects with commands.

Testing Instructions

php cli/joomla.php list

note that database:import/export is not covered by this PR as that is a library of its own.

Actual result BEFORE applying this Pull Request

Screenshot 2021-01-09 at 13 06 35

Expected result AFTER applying this Pull Request

Screenshot 2021-01-09 at 13 06 49

And after installing extensions with other commands you can see why this is much neater and makes sense to do:

Screenshot 2021-01-09 at 13 11 33

Documentation Changes Required

Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
@PhilETaylor

This comment was marked as abuse.

@wilsonge
Copy link
Contributor

Hmm I kinda get the aim of this. But at minimum the users and finder things are their own extensions so should be as top level as any other 3rd party extension.

Honestly I'm unsure on grouping session/site/extension commands together. I'm going to talk around the maintainers group for opinions.

@ceford
Copy link
Contributor

ceford commented Jan 10, 2021

I have tested this item ✅ successfully on e84dcd8

It works as described. Whether it is the right thing to do is up to others.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31941.

@alikon
Copy link
Contributor

alikon commented Jan 10, 2021

i don't like it
too much verbose
my 0.02€

@PhilETaylor

This comment was marked as abuse.

@ghost
Copy link

ghost commented Jan 11, 2021

I have tested this item ✅ successfully on e84dcd8


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31941.

@PhilETaylor

This comment was marked as abuse.

@wilsonge
Copy link
Contributor

The CLI App interface isn't compatible with Joomla's (the default parameter in the CMSApplication->get() method so we adapted most of it)

@brianteeman
Copy link
Contributor

As long as all add-ons namespace their commands (as akeeba has done) I'm not sure I see where the problem lies

@laoneo
Copy link
Member

laoneo commented Jan 28, 2021

Sooner or later it will clash, so better to namespace them yet.

@himanshu007-creator
Copy link
Contributor

himanshu007-creator commented Feb 24, 2021

I have tested this item ✅ successfully on e84dcd8

After applying the patch. It works as mentioned.
A small correction, in the testing instructions, please change the command php clli/joomla.php list to php cli/joomla.php list


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31941.

@laoneo
Copy link
Member

laoneo commented Feb 25, 2021

I have tested this item ✅ successfully on e84dcd8

Run a couple of commands like php cli/joomla.php joomla:session:gc. All of them worked without an issue.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31941.

@wilsonge
Copy link
Contributor

Ok I've talked in maintainers a little bit. I'm going to close this - I think we've roughly got the right breakdown already - where these are roughly broken down by our internal component names. We can probably group some things together like maybe update and core or similar but I think everything under one name is way too far.

@wilsonge wilsonge closed this Feb 25, 2021
@PhilETaylor

This comment was marked as abuse.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants