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

Deep strict validation #10

Conversation

NathanielWroblewski
Copy link
Contributor

@NathanielWroblewski NathanielWroblewski commented Jul 28, 2016

@mike-bourgeous Adds deep strict validation of schemas to address #9

Open to feedback and changes.

@mike-bourgeous
Copy link
Collaborator

@NathanielWroblewski This looks interesting. I'll have time next week to explore the idea in more detail.

What do you think of the idea of doing away with shallow strict validations and making strictness deep always? Fewer methods and parameters to pass around that way, and fewer edge cases to test (like making sure shallow checks really are shallow). Maybe with a minor version bump.

@NathanielWroblewski
Copy link
Contributor Author

NathanielWroblewski commented Aug 1, 2016

I think that makes sense. My initial expectation was that validate_strict would run deeply, so I think making that method deep by default would be less surprising.

I also share your concern that those changes could break existing implementations, that's why I added this as a separate method. But if you're considering a version bump with these changes, and as long as the interface supports an option for running shallow validations in strict mode for users who need time to update or who do not wish to update, I think that would be a better change.

Let me know if I can be of any help.

@NathanielWroblewski
Copy link
Contributor Author

One other concern that may be worth mentioning, is the increasing arity of the validate/validate_strict methods and their order not matching here. It would perhaps be better to pass a hash of options:

ClassyHash.validate_strict(hash, schema, 
  shallow: true,
  verbose: true
)

@mike-bourgeous
Copy link
Collaborator

mike-bourgeous commented Aug 3, 2016

I'm considering keyword arguments to avoid arity explosion and to avoid Hash allocation.

mike-bourgeous added a commit that referenced this pull request Aug 10, 2016
…ct-validation

Deep strict validation (into v0.2.0 branch)

Closes #9
Closes #10
@mike-bourgeous
Copy link
Collaborator

Superseded by #15

@NathanielWroblewski
Copy link
Contributor Author

Thank you for the great work on this! I really appreciate the rapid progress! 👍 on the new interface

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.

None yet

2 participants