-
Notifications
You must be signed in to change notification settings - Fork 29
Refactored node type register command to be more flexible re. loading #108
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i was going to say this is jackrabbit specific but then i found http://www.day.com/maven/jcr/2.0/25_Appendix.html#25.2%20Compact%20Node%20Type%20Definition%20Notation which indicates its now somehow part of jcr. did not find it in the chapter on node type management...
|
Updated, keeping backwards compatibility, but explicitly marking as depreacated. |
|
ok, updated to use an argument array instead of options, thanks @dbu |
|
... and fixed the tests. |
|
i think just passing the list as arguments rather than parameter is fine for the new syntax. but we should keep the cnd-file parameter for BC compatibility and only drop it on a new major release. the tests seem not really fixed. |
|
ok makes sense, reverted the name of the argument. The failing test does not seem to be related to this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could depend on the symfony file component, would be much nicer. but i would say its not worth the additional dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah
|
can you please update the doc? then its good to merge imho. |
|
Updated doc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we should say here that it can also be folders and that you can specify several files / folders. (btw, is it a deep recursion or just the folder itself?) folder itself is fine, but we should say it.
|
looking good. could you add a bit to the doc, and then squash your commits? |
- Allow multiple files to be specified - Allow folders to be specified
|
Updated + Squashed |
Refactored node type register command to be more flexible re. loading
This PR:
Its a BC break in that it forces at least one
targetoption to be secified, whereas before there was a required argument for a single CND file.This PR also keeps in mind support for YAML files instead of CND (I am currently working on a YAML importer + dumper).