Fixes #32099: Check if an Integer for integer data_type #326
Conversation
This file contains 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
wbclark
approved these changes
Mar 15, 2021
|
hmmm, is happening somehow in test/kafo/data_types/hash_test.rb ? |
|
Hrmm, means this method is not used how I thought it was used. I'll take another crack at modifying the original. |
ehelms
changed the title
|
Updated. I did change one assertion because it seemed odd to me that "1foo" should be consider an Integer and return "1". If you mix letters and numbers, that ought to be a string. An integer should be clearly an integer. |
ekohl
reviewed
Mar 15, 2021
lib/kafo/data_types/integer.rb
Outdated
| @@ -19,7 +19,9 @@ def to_s | |||
| end | |||
|
|
|||
| def typecast(value) | |||
| value =~ /\d+/ ? value.to_i : value | |||
| Integer(value) | |||
| rescue | |||
There was a problem hiding this comment.
Isn't it better to catch ArgumentError explicitly? Catching all exceptions is generally a bad practice.
|
Additional to this, I think having theforeman/jenkins-jobs#5 in place would help catch these outside of runtime testing. |
|
This required some additional cleanup to get everything to work properly. |
ekohl
approved these changes
Mar 16, 2021
…ta_type This fixes a deprecation warning present in Ruby 2.7: warning: deprecated Object#=~ is called on Integer; it always returns nil The change removes Kafo's custom TypeError exception as there was no use of it inside the project and it had the effect of shadowing the built in Ruby TypeError.
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.
This fixes a deprecation warning present in Ruby 2.7:
warning: deprecated Object#=~ is called on Integer; it always returns nil