Skip to content

Conversation

dantleech
Copy link
Member

This is just an proof of concept PR, so basically I propose to renmae the classes as follows:

./NodeMoveCommand.php
./NodeRemoveCommand.php
./NodeTouchCommand.php
./NodeTypeListCommand.php
./NodeTypeRegisterCommand.php
./WorkspaceCreateCommand.php
./WorkspaceDumpCommand.php
./WorkspaceExportCommand.php
./WorkspaceImportCommand.php
./WorkspaceListCommand.php
./WorkspacePurgeCommand.php
./WorkspaceQueryCommand.php

and that would then translate to the following command schema:

phpcr:workspace:purge
phpcr:workspace:import
phpcr:workspace:query
...
phpcr:node-type:register
...
phpcr:node:touch
...

Not sure if I have categorized things correctly, does dump,import|export etc belong in workspace?

@lsmith77
Copy link
Member

lsmith77 commented May 3, 2013

looks good to me.

@dbu
Copy link
Member

dbu commented May 3, 2013

yes, you got them right. i wonder if we should leave out :workspace: from the commands that are about a workspace. in the end, all node commands are about workspaces as well, everything except node types is. having phpcr:purge, phpcr:import / export, phpcr:query would be more compact than the "workspace" in the middle.

wdyt?

@dbu
Copy link
Member

dbu commented May 3, 2013

@dbu said: well but the node comands also only apply to the node in that workspace
@lsmith77 said: one could therefore also argue workspace:node:move ..

but that would be painful so i argue for shorter names. but whatever, can live with the longer names too.

@dantleech
Copy link
Member Author

Hmm, the problem I think is that we need the workspace prefix for create and list. But it then follows logically that the others are also in that prefix. With that scheme the only thing that bugs me is query as its something potentially used often, but for me it would be a schematic shame to separate it from workspace ..

In addition I think workspace:dump should be changed to node:dump - workspace:dump infers that we always dump the entire workspace.

wdyt?

@dbu
Copy link
Member

dbu commented May 3, 2013

agreed on all points. i prefer a bit longer over inconsistent.

@dbu
Copy link
Member

dbu commented May 3, 2013

i will tag things now as otherwise jackalope is not installable. but i will just cheat and update the tag on phcpr-utils after merging this PR...

@dantleech
Copy link
Member Author

Updated .. have separated workspace:purge and node:remove - you cannot purge "/" from node:remove and you cannot purge a path from workspace:purge.

I was intending to push some tests for all the commands too, but I had already been doing that for 2 hours and only have 2.5 tests done :) Might see if I can do functional tests instead - as such I havn't actually tested all these changes.

@dbu
Copy link
Member

dbu commented May 4, 2013

very cool, thanks. and indeed good idea to separate purge and remove, avoids accidental data complete loss. i will change the remove path argument to be required and no default value after merging the PR.

dbu added a commit that referenced this pull request May 4, 2013
[RFC] Reorganizing command names
@dbu dbu merged commit 43e06b7 into phpcr:master May 4, 2013
@dantleech
Copy link
Member Author

I guess the DoctrinePHPCRBundle needs updating now?

@lsmith77
Copy link
Member

lsmith77 commented May 4, 2013

yes

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.

3 participants