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

fixes #10232 - moving validations and casting out of lookup key and value #2399

Conversation

unorthodoxgeek
Copy link
Member

No description provided.

@domcleal
Copy link
Contributor

Please check the rubocop and unit test failures, thanks.

context "failues" do
test "caster cowardly doesn't make any change in case it doesn't know how to cast" do
item = OpenStruct.new(:foo => "blah")
#assert_raises TypeError do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented code here and below

@domcleal
Copy link
Contributor

@ares would you mind giving this look over, since you've spent some time in this area? It looks minimally invasive to me, just moving code rather than behavioural changes.

@@ -0,0 +1,118 @@
module Parameters
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we put this under the Foreman namespace too? e.g. Foreman::Parameters::Caster

module Parameters
class Caster
TRUE_VALUES = [true, 1, '1', 't', 'T', 'true', 'TRUE', 'on', 'ON', 'yes', 'YES', 'y', 'Y'].to_set
FALSE_VALUES = [false, 0, '0', 'f', 'F', 'false', 'FALSE', 'off', 'OFF', 'no', 'NO', 'n', 'N'].to_set
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both unused?

@domcleal
Copy link
Contributor

Please note that not all of my previous comments were addressed.

@@ -214,7 +207,7 @@ def array2path(array)
def validate_and_cast_default_value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misnamed, should be cast only.

@unorthodoxgeek
Copy link
Member Author

@domcleal - fixed your comments

@@ -6,6 +6,7 @@
factory :environment_class

factory :lookup_key do
key_type "string"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also not required.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@orrabin orrabin force-pushed the 10232-move-validations-and-casting branch from 93c64c4 to a07a28a Compare August 18, 2015 13:39
@orrabin
Copy link
Member

orrabin commented Aug 18, 2015

@domcleal I rebased and fixed the factory.

@domcleal
Copy link
Contributor

Merged as 9f88e8a, thanks @unorthodoxgeek and @orrabin.

@domcleal domcleal closed this Aug 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants