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

Fix issue where package provider override isn't working when specifying a package_source #18

Merged
merged 6 commits into from
Jun 10, 2015

Conversation

irvingpop
Copy link
Contributor

Per the comments in chef/chef#3487 - passing a provider to the package resource is unreliable and causing problems for our customers that need to specify a package_source

This PR also adds tests for the test::local recipe and gets up to 100% test coverage 😄

cc: @jtimberman

Irving Popovetsky added 2 commits June 4, 2015 16:48
@jtimberman
Copy link
Contributor

Is this still required, given your last comment chef/chef#3487?

@irvingpop
Copy link
Contributor Author

Hey @jtimberman - Lamont chimed in with a much nicer implementation using declare_resource - and provided a more in-depth explanation about why not using provider is a good idea: chef/chef#3487 (comment)

I updated this PR to use declare_resource and it's much readable than before. Also it has some awesome tests :)

I guess ultimately it comes down to how you feel about promoting the declare resource pattern over the provider Chef::Package::Provider::Rpm pattern.

return Chef::Provider::Package::Rpm if node['platform_family'] == 'rhel'
def local_package_resource
return :dpkg_package if node['platform_family'] == 'debian'
return :rpm_package if node['platform_family'] == 'rhel'

Choose a reason for hiding this comment

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

should probably return :package here by default, or else raise if you don't want to try to guess that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, done

@lamont-granquist
Copy link

The declare_resource bits LGTM 👍

@@ -1,5 +1,5 @@
name 'chef-server-ingredient'
version '0.3.2'
version '0.3.3'
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not increment versions in PR's 🍰

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry about that! fixed.

jtimberman added a commit that referenced this pull request Jun 10, 2015
Fix issue where package provider override isn't working when specifying a package_source
@jtimberman jtimberman merged commit d880559 into chef-boneyard:master Jun 10, 2015
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

3 participants