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/nil default options #48

Closed

Conversation

dldinternet
Copy link
Contributor

Recreated the pull request after fixing the check.

The idea is to check the configuration values returned and to not set options that don't have default values.

johnbellone added a commit to johnbellone/knife-server that referenced this pull request Mar 17, 2014
This is same fix as fnichol#48 expect applying it a little bit better to all of
the different provisioners.
johnbellone added a commit to johnbellone/knife-server that referenced this pull request Mar 17, 2014
This is same fix as fnichol#48 expect applying it a little bit better to all of
the different provisioners.
johnbellone added a commit to johnbellone/knife-server that referenced this pull request Mar 17, 2014
This is same fix as fnichol#48 expect applying it a little bit better to all of
the different provisioners.
@@ -72,7 +72,8 @@ def ec2_bootstrap
ENV['NO_TEST'] = "1" if config[:no_test]
bootstrap = Chef::Knife::Ec2ServerCreate.new
Chef::Knife::Ec2ServerCreate.options.keys.each do |attr|
bootstrap.config[attr] = config_val(attr)
val = config_val(attr)

Choose a reason for hiding this comment

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

Indentation doesn't match

@josephholsten
Copy link

This bug effects all providers: EC2, Linode and standalone. It should really be extracted up into a parent or out into a module.

fnichol added a commit that referenced this pull request Sep 3, 2014
@fnichol
Copy link
Owner

fnichol commented Sep 3, 2014

@josephholsten you nailed it, affected all other knife plugins which 60c68e8 should address.

@dldinternet I rebased this PR and merged in into master with b2520f4 . Thank you very much!!

@fnichol fnichol closed this Sep 3, 2014
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.

3 participants