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

schema.coerce - support String -> s/Bool [0.2.1] #81

Closed
hlship opened this issue Apr 14, 2014 · 4 comments
Closed

schema.coerce - support String -> s/Bool [0.2.1] #81

hlship opened this issue Apr 14, 2014 · 4 comments

Comments

@hlship
Copy link

hlship commented Apr 14, 2014

Neither json-coercion-matcher nor string-coercion-matcher will coerce a string value to a boolean.

This is desirable for me, as sometimes I'm apply schema to query parameters and want to convert "true" and "false" to true and false.

@w01fe
Copy link
Member

w01fe commented Apr 15, 2014

Sounds good to me -- PR welcome, or we'll get to it as soon as we can. Thanks!

davegolland added a commit that referenced this issue May 7, 2014
adding support for coercing booleans (issue #81)
@davegolland
Copy link
Member

Fixed in #88.

@semperos
Copy link

@davegolland @w01fe Just my $0.02, thanks for all your excellent work on this library.

The string->boolean coercion is included with JSON's coercions by default. JSON, however, already specifies true and false as legal values.

Given this, I think it's a mistake to include a default JSON coercion that transforms strings to booleans. It propagates sloppy JSON usage and becomes a source of hiding developer mistakes.

It would be easy enough for consumers of Schema's coercions to add this coercion explicitly if they need to support JSON payloads that don't use proper true and false values.

@w01fe
Copy link
Member

w01fe commented Oct 30, 2015

Good point, I think this should probably have been added to string coercions, since it's not consistent with the rest of the defaults (e.g., number from string is also not part of JSON). That said, someone could be using this productively now (e.g., for boolean keys in a map) so we'd have to treat it as a breaking change. PR welcome! Thanks for your $0.02 :)

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

No branches or pull requests

4 participants