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

Feature/attr apply #10

Merged
merged 6 commits into from Mar 23, 2015
Merged

Feature/attr apply #10

merged 6 commits into from Mar 23, 2015

Conversation

realloc
Copy link
Contributor

@realloc realloc commented Mar 4, 2015

Adding recipe to define ssl_certificate resources from node attributes.

@zuazo
Copy link
Owner

zuazo commented Mar 9, 2015

Thanks @realloc for your PR.

Could you fix the RuboCop offenses and add some tests (test-kitchen or ChefSpec)?

@realloc
Copy link
Contributor Author

realloc commented Mar 22, 2015

Fixed rubocop. Added simple ChefSpec test.

chain_encrpted chain_secret_file chain_content
chain_combined_name chain_combined_path ca_cert_path ca_key_path
).each do |attr|
send(attr, item_hash[attr]) if item_hash[attr]
Copy link
Owner

Choose a reason for hiding this comment

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

If I understand this correctly, something like this should do the same:

node['ssl_certificate']['items'].each do |item_hash|
  ssl_certificate item_hash[:name] do
    namespace item_hash
  end
end

Am I missing something?

See the namespaces documentation.

@realloc
Copy link
Contributor Author

realloc commented Mar 23, 2015

Yes, your code does the same thing better.
I missed namespaces, was thinking about filtering attribute and allowing to set only pre-defined set of them.

zuazo added a commit that referenced this pull request Mar 23, 2015
@zuazo zuazo merged commit a04beeb into zuazo:master Mar 23, 2015
zuazo added a commit that referenced this pull request Mar 29, 2015
@zuazo
Copy link
Owner

zuazo commented Apr 5, 2015

Released in 1.4.0. Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants