-
Notifications
You must be signed in to change notification settings - Fork 583
(19864) num2bool match fix #139
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
(19864) num2bool match fix #139
Conversation
Also ignore rspec fixtures directory
No more coercing to String and regex matching. Instead, we now coerce to Integer at the beginning or raise an error if we cannot coerce to Integer. A consequence of this change is that the function will now accept blatantly non-numeric strings as input, and return false. This seems a bit goofy to me, but it's how String#to_i works. If we really don't like this, then I'm open to suggestions.
This is a bit more heavy-handed than I might like, but it does appear to do the right things: * accepts numeric input appropriately, truncating floats * matches string input against a regex, then coerces number-looking strings to int * makes a best effort to coerce anything else to a string, then subjects it to the same treatment * raises an error in the event of incorrect number of arguments or non-number-looking strings I've also included some additional unit tests.
Um, ok, now I'm even more confused. The Travis build succeeded for Puppet 3 and Puppet 2.7 on Ruby 1.8.7 (which is what I use), but failed on Ruby 1.9.3. Off the top of my head I have no idea what's going on there; any suggestions? |
number = number.to_i | ||
|
||
# Return true for any positive number and false otherwise | ||
return number > 0 ? true : false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
number > 0
will return a boolean, so you don't need to use the ternary operator here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's straightforward enough; fixed.
@hakamadare I initially thought it was an issue with the regex, and that using |
* use Float() to process string arguments * get rid of doubly nested arrays * removing needless ternary operator * improving error message handling
Merged into master as f90c54e. This should be released in the next major release of stdlib. Thanks again for the contribution! -Adrien |
This is another attempt at #137.
This is a bit more heavy-handed than I might like, but it does appear to do the right things:
I've also included some additional unit tests.