Skip to content

Use elements local property to generate proper JMS mappings#108

Merged
goetas merged 1 commit intogoetas-webservices:masterfrom
Toilal:element-local-property
Aug 24, 2020
Merged

Use elements local property to generate proper JMS mappings#108
goetas merged 1 commit intogoetas-webservices:masterfrom
Toilal:element-local-property

Conversation

@Toilal
Copy link
Copy Markdown
Contributor

@Toilal Toilal commented Feb 12, 2020

composer.json Outdated
},
"scripts": {
"tests": [
"php ./vendor/phpunit/phpunit/phpunit -v"
Copy link
Copy Markdown
Member

@goetas goetas Jun 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think that this is not needed. you can do just vendor/bin/phpunit (is short and works the same)

(this is just a minor note, no need to fix it for now)

@Toilal Toilal force-pushed the element-local-property branch 2 times, most recently from 697a496 to 9a19839 Compare August 21, 2020 22:18
@Toilal
Copy link
Copy Markdown
Contributor Author

Toilal commented Aug 21, 2020

I'm working right now to narrow down the new test.

@goetas
Copy link
Copy Markdown
Member

goetas commented Aug 22, 2020

👍

@goetas
Copy link
Copy Markdown
Member

goetas commented Aug 22, 2020

The part with the xsd reader has been merged, now I think that should be easier to work on this

@Toilal
Copy link
Copy Markdown
Contributor Author

Toilal commented Aug 22, 2020

Let's try with @dev on xsd-reader.

@Toilal
Copy link
Copy Markdown
Contributor Author

Toilal commented Aug 22, 2020

COMPOSER_FLAGS='--prefer-lowest --prefer-stable' build is failing because it doesn't grab the @dev version of xsd-reader, but other builds are OK.

@Toilal
Copy link
Copy Markdown
Contributor Author

Toilal commented Aug 22, 2020

Maybe you could perform a release of xsd-reader, so we can merge this pull request with a stable version constraint on this dependency ? Then xsd2php could be released too ? PR on soap-client is only related to tests, so it can wait.

@goetas
Copy link
Copy Markdown
Member

goetas commented Aug 22, 2020

@Toilal Toilal force-pushed the element-local-property branch from 40b7d8b to 757f288 Compare August 22, 2020 19:38
@Toilal
Copy link
Copy Markdown
Contributor Author

Toilal commented Aug 22, 2020

Thanks for the release of xsd-reader. Now, this PR should be merged and xsd2php released too.

// 'xml_element' => array(
// 'namespace' => 'http://www.example.com'
// ),
'xml_element' => array(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the whole test? do we need something better?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don't understand your question. What's the problem here ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ho I see. You mean the test changes are too short for the fix ? I'll try to add more checks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, the fix mostly relies on xsd-reader changes. xsd-reader carries a whole new test class for the use case, so i'm not sure it's required to bring more checks in xsd2php module.

@goetas goetas merged commit b77fd30 into goetas-webservices:master Aug 24, 2020
@goetas
Copy link
Copy Markdown
Member

goetas commented Aug 24, 2020

I'm good with it! Thanks a lot for the hard work!
I have to admit that this libraries needed some maintenance, and you did it! Thanks again

@Toilal Toilal deleted the element-local-property branch August 24, 2020 16:47
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.

2 participants