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

Empty optional nested array #1384

Closed
wants to merge 4 commits into from

Conversation

ipkes
Copy link
Contributor

@ipkes ipkes commented May 3, 2016

Based on issue and tests

  1. Some test modification
  2. Possible solution

@ipkes ipkes force-pushed the empty_optional_nested_array branch from a200ac9 to 2eda1a9 Compare May 3, 2016 16:14
@ipkes
Copy link
Contributor Author

ipkes commented May 4, 2016

I'm afraid solution is insufficient and needs some work here. A major problem here with digging deeper in array. Need more time to think

@dblock
Copy link
Member

dblock commented May 4, 2016

Start by rebasing and squashing this, there're a bunch of unrelated changes, hard to grok.

@ipkes ipkes force-pushed the empty_optional_nested_array branch from 2eda1a9 to 068dfbf Compare May 5, 2016 18:48
@ipkes
Copy link
Contributor Author

ipkes commented May 5, 2016

  1. correctly done rebasing
  2. another spec modify, more complex example (to digging deeper)
  3. and sufficient solution
    problem was with checking on blank, first sometimes we have already blank parameter and we should check it and second we should check on blank not for all elements, just for any.

@ipkes
Copy link
Contributor Author

ipkes commented May 6, 2016

And for information. When I tried to find solution, I analyzed elements and parameters, which came, without complete understanding why they are coming. I can't be sure that anything else parameters will be work as well, but to already described parameters, it's ok.

@dblock
Copy link
Member

dblock commented May 6, 2016

This makes total sense to me. We want to do an allow_blank validation if anything is blank here. Merging.

@dblock
Copy link
Member

dblock commented May 6, 2016

Merged via 9a08358, thank you.

@dblock dblock closed this May 6, 2016
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