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

Honor settings in recipes #25

Merged
merged 2 commits into from
Sep 10, 2014
Merged

Honor settings in recipes #25

merged 2 commits into from
Sep 10, 2014

Conversation

johnewart
Copy link
Contributor

If the user has specified options in with_chef_local_server or other setting as symbols then honor those. Fixes #163

end

# Copy over to symbol key for things that use symbol keys...
options[symbol_key] = options[string_key]

Choose a reason for hiding this comment

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

I'm concerned that this will overwrite the symbol value with the string value, which is the opposite behavior of cheffish 0.7.x: https://github.com/opscode/cheffish/blob/v0.7.1/lib/cheffish/recipe_dsl.rb#L71

In my example I was passing :cookbook_path in and a "cookbook_path" was getting set automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, the string value does take precedence, so if you have "cookbook_path" and :cookbook_path, then "cookbook_path" will win. I will flip-flop them for consistency with previous behavior ;) It's worth noting that your example will still work though, because if you don't have a string key of the same name, the symbol key will get propagated to the string key, and then all the merged values as string keys will also get set as symbol keys.

Choose a reason for hiding this comment

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

We could also do

options[symbol_key] = options[string_key] unless options[symbol_key]

to preserve the symbol_key if it isn't nil

johnewart added a commit that referenced this pull request Sep 10, 2014
@johnewart johnewart merged commit 0781734 into master Sep 10, 2014
@thommay thommay added Bug Type: Bug Does not work as expected. and removed Bug labels Jan 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Does not work as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants