(19864) Enable num2bool to accept numeric input #137
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 4.x: Revert "Revert "Revert "Revert "Merge branch 'hkenney-ticket/master/2157_remove_facts_dot_d'"""" Revert "Revert "Revert "Merge branch 'hkenney-ticket/master/2157_remove_facts_dot_d'"""
* 4.x: Update Modulefile, CHANGELOG for 3.1.0
* 4.x:
* 4.x: Revert "Revert "Merge branch '2.5.x' into 3.0.x"" Revert "Merge branch '2.5.x' into 3.0.x" (maint) Fix spec failures resulting from Facter API changes
* 4.x: Update Modulefile, CHANGELOG for 2.5.1
* 4.x: Add function, uriescape, to URI.escape strings. Redmine #17459 Add function, uriescape, to URI.escape strings. Redmine #17459 Add function, uriescape, to URI.escape strings. Redmine #17459 Update CHANGELOG, Modulefile for 3.1.1
* 4.x: (#13974) Add predicate functions for interface facts
* 4.x: Add the pick() function
* 4.x: Extend delete function for strings and hashes Fixed typo
* jfryman-master: puppet-lint cleanup
* 4.x: Add join_keys_to_values function
* 4.x: (#14670) Fixup file_line autorequire specs (#14670) autorequire a file_line resource's path
* 4.x: (#17797) min() and max() functions
* 4.x: (Maint) Add spec/functions to rake test task Add example behaviors for ensure_packages() function Add an ensure_packages function.
* 4.x: Revert "(Maint) Add spec/functions to rake test task"
* 4.x: Add reject() function
* 4.x: Update Modulefile, CHANGELOG for 2.6.0
* 4.x: Update Modulefile, CHANGELOG for 3.2.0
* dalen-style_fixes: maint: style guideline fixes
Minor clarification to the compatibility section of the README.
* maint/master/update_readme: Clarify that stdlib 3 supports Puppet 3
* 4.x: Add test/validation for is_float if created from an arithmetical operation Add test/validation for is_integer if created from an arithmetical operation Add test/validation for is_numeric if created from an arithmetical operation
Without this patch stdlib has Travis CI configuration files, but they don't seem to completely specify the dependency versions and the build matrix. This patch addresses the problem by putting the dependency information in the conventional Gemfile location. This patch should coincide with enabling Travis CI support for pull requests. A build status image is also included in the project README.
* feature/master/travis_ci: (maint) Add Travis CI Support
Without this patch we're not building against ruby head. This is a problem because we need to know if standard lib works with the latest version of MRI. This patch builds against ruby head but also allows the build to pass if there are failures with ruby-head.
Future proof travis build matrix
The exclude keyword was accidentally specified twice.
Fix typo in travis configuration
Add missing documentation for validate_augeas and validate_cmd to README.markdown
Copied from the same one included in Puppet and Facter. [ci skip]
The function only uses the first argument, so raise an error with too few arguments *and* with too many arguments.
This function provides a simple wrapper around Puppet::Parser::Functions.function for access within Puppet manifests. This will allow users to check whether or not a plugin or functionality such as hiera is installed on the server.
This closes puppetlabsGH-59
Add floor function implementation and unit tests
Since the function tries to match the argument against a regex, first coerce the argument to a string so that the match can happen. Also includes some additional unit tests.
|
CLA Signed by hakamadare on 2012-01-17 21:00:00 -0800 |
| @@ -14,7 +14,8 @@ module Puppet::Parser::Functions | |||
| raise(Puppet::ParseError, "num2bool(): Wrong number of arguments " + | |||
| "given (#{arguments.size} for 1)") if arguments.size < 1 | |||
|
|
|||
| number = arguments[0] | |||
| # Since we're matching against a regex, coerce to String | |||
| number = arguments[0].to_s | |||
There was a problem hiding this comment.
This is going to cast a number to a string and do a regex on it, but we already have the number. It would be better to check of arguments[0] is a number and if so do the logic on that number.
|
Thank you for this contribution! I see the value in this change, but it seems that the implementation could be improved a bit. I've attached an inline comment on where I think things should be changed. Once these concerns are addressed I think we can merge this in. Thanks again! |
|
i'm fine with getting rid of the regex, but i think technically what we want is to know whether |
|
If the function is passed a string I'm fine with doing a regex, but if we already have a numeric value there's no reason to cast to a string; we can just check the value immediately. Switching on the class of case arguments[0]
when String
# do string things
when Numeric
# Do numeric things
else
# unhandled value; try to coerce to a string
end |
When prefix and suffix did error checking with positional arguments, they would not report the position of the argument that failed to validate. This commit changes the messages to indicate which argument failed.
This closes puppetlabsGH-138.
|
I noticed this hasn't had any updates in a week. I'd like to keep this open because I'd hate to not resolve this issue, do you think you'll have a chance to address the next actions sometime this week? |
Conflicts: .gitignore
Since the function tries to match the argument against a regex, first coerce the argument to a string so that the match can happen. Also includes some additional unit tests.
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.
…etlabs-stdlib into 19864_num2bool_match_fix Conflicts: lib/puppet/parser/functions/num2bool.rb spec/unit/puppet/parser/functions/num2bool_spec.rb
|
oh ffs. that rebase did not do what i had expected. let me try to clean this up. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since the function tries to match the argument against a regex, first
coerce the argument to a string so that the match can happen. Also
include some additional unit tests.