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

Use .single() option for directory settings schema #14

Merged
merged 1 commit into from Dec 4, 2014

Conversation

@kanongil
Copy link
Member

kanongil commented Dec 4, 2014

No description provided.

@kanongil

This comment has been minimized.

Copy link
Member Author

kanongil commented Dec 4, 2014

Note that doing actual validation means that the boolean values will be handled slightly different for string input. Previously, "false" would pass the assertion check but actual usage evaluates it to true.

@hueniverse

This comment has been minimized.

Copy link
Member

hueniverse commented Dec 4, 2014

Is this a bug fix then? or breaking change?

@kanongil

This comment has been minimized.

Copy link
Member Author

kanongil commented Dec 4, 2014

Good question. I did this as a refactor, and only realized the slightly changed behavior afterwards.

The issue is actually a bit trickier than it would seem. I'm inclined towards labelling the non-assertion for Joi.assert("false", Joi.boolean()) as a bug in joi, as it will likely produces weird results without conversion as well. I'm not sure how you would fix it though. Maybe @Marsup can provide some insight?

@hueniverse

This comment has been minimized.

Copy link
Member

hueniverse commented Dec 4, 2014

'false' is a valid boolean value in joi with the default settings. The problem here is that the value is allowed but the casting is not preserved leading to invalid settings. I think the change is more about Joi.assert(). Either we change the defaults to strict mode, or change how we use it by keeping the validated result.

Either way, I would call this a bug fix for inert.

@hueniverse hueniverse added the bug label Dec 4, 2014
@hueniverse hueniverse self-assigned this Dec 4, 2014
@hueniverse hueniverse added this to the 2.0.0 milestone Dec 4, 2014
hueniverse added a commit that referenced this pull request Dec 4, 2014
Use .single() option for directory settings schema
@hueniverse hueniverse merged commit e5afc5c into hapijs:master Dec 4, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
@kanongil kanongil deleted the kanongil:joi-usage branch Dec 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.