Skip to content
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

Fixed NodeTypeDefinition::fromXml + added a test for it #203

Merged
merged 2 commits into from Jan 20, 2014

Conversation

Projects
None yet
3 participants
@wjzijderveld
Copy link
Contributor

wjzijderveld commented Jan 18, 2014

Creating a NodeTypeDefinition from XML didn't work because it wasn't converted to an array before passing it to the ItemDefinition constructor.

@lsmith77

This comment has been minimized.

Copy link
Member

lsmith77 commented Jan 18, 2014

my favorite kind of PR: reduces code in the lib, adds a test :)

I will leave it to @dbu to decide on the merge ..

@dbu

This comment has been minimized.

Copy link
Member

dbu commented Jan 20, 2014

looks like we should run a php mess detector and check if we have more copy-paste code somewhere. looks a lot like somebody factored out that code into the converter class but then forgot to use that class.

i just tested with jackalope-jackrabbit and seems fine too.

thanks a lot!

dbu added a commit that referenced this pull request Jan 20, 2014

Merge pull request #203 from wjzijderveld/nodetypedefinition-fromxml
Fixed NodeTypeDefinition::fromXml + added a test for it

@dbu dbu merged commit 0206286 into jackalope:master Jan 20, 2014

1 check passed

default Scrutinizer: Errored — Travis: Passed
Details

@wjzijderveld wjzijderveld deleted the wjzijderveld:nodetypedefinition-fromxml branch Jan 21, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.