remove explicit include of recipe chef_sugar::default

davidalpert commented 6 years ago

Description

This PR removes the outdated include 'chef_sugar' line from recipe[firewal_default].

Since v4.0.0. of the chef_sugar it is no longer required to include this recipe explicitly and doing so generates warnings as described below.

Issues Resolved

the following warning is currently logged each time someone converges with a recipe which includes or depends on recipe[firewall:default]:
```
WARN: chef-sugar::default no longer needs to be included in your runlist.
      Instead simply depend on the chef-sugar cookbook and the gem will
      be intalled and loaded automatically.
```
this can happen once for each chefspec which uses the chef_run pattern creating a lot of repetative noise in rspec output.

Check List

[x] All tests pass. See https://github.com/chef-cookbooks/community_cookbook_documentation/blob/master/TESTING.MD
[x] New functionality includes testing (no new functionality).
[x] New functionality has been documented in the README if applicable (no new functionality)
[x] All commits have been signed for the Developer Certificate of Origin. See https://github.com/chef-cookbooks/community_cookbook_documentation/blob/master/CONTRIBUTING.MD

martinb3 commented 6 years ago

Hi there -- thank you for submitting this! I think we need to pin the chef-sugar dependency to >= 4.0.0 for this to work, and it'll also be a backwards incompatible change for any downstream users who were using this cookbook but now would be forced to upgrade chef-sugar. Do you agree with those two items? Thanks again!

davidalpert commented 6 years ago

@martinb3 I think you are correct that this would require a version pin; I hadn't thought of that. we try to run without pins as much as possible so bringing in the latest chef-sugar gem is what brought on the storm of warnings in our specs.

I will defer to you and the other maintainers as to when is a good time to add such a pin on chef-sugar to this cookbook.

martinb3 commented 6 years ago

Thanks @davidalpert -- I'll leave this open, and merge it when we next do a major bump anyway. 👍

davidalpert commented 6 years ago

@martinb3 I discussed this PR with @swalberg and he suggested that I can add a guard to only apply this when the dependent cookbook is >= 4.0.0 so I will look into that.

davidalpert commented 6 years ago

@martinb3 would that last change allow us to merge this earlier, before a major bump to this cookbook?

martinb3 commented 6 years ago

Thanks! I'll release this now as v2.6.4.

sous-chefs / firewall