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

implement @SkipWhenEmpty for strings #883

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

leoric
Copy link

@leoric leoric commented Mar 7, 2018

Q A
Bug fix? no
New feature? yes
Doc updated no
BC breaks? no
Deprecations? no
Tests pass? yes
License Apache-2.0

@goetas
Copy link
Collaborator

goetas commented Mar 7, 2018

Hi! Thanks for the contribution.
Can you update the other visitors? (xml and yaml)
Can you also update the documentation?

return;
}
if ('' === $v) {
return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

indentation with 4 spaces here

Новоселов Андрей added 2 commits March 7, 2018 17:11
@leoric
Copy link
Author

leoric commented Mar 7, 2018

fixed formatting, made documentation more clear. Xml visitor don't need an update - you can't create an empty string node in xml. Yaml visitor doesn't handle this tag at all ATM

@goetas goetas self-assigned this Mar 23, 2018
@goetas goetas added this to the v1.12 milestone Mar 23, 2018
@goetas goetas removed this from the v1.12 milestone May 3, 2018
@goetas goetas removed their assignment May 3, 2018
@goetas goetas removed the To Merge label May 10, 2018
@TNAJanssen
Copy link

When can this be merged?

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

Successfully merging this pull request may close these issues.

3 participants