Fix deprecation warnings #123

Merged
merged 2 commits into from Sep 25, 2016

Conversation

Projects
None yet
3 participants
@operatingops
Contributor

operatingops commented Sep 24, 2016

node.set causes deprecation warnings in tests (found while writing a cookbook to setup a bastion SSH server).

I changed the specs to node.normal because that’s what the ChefSpec readme now recommends (a change made by the maintainer after getting reports of the same deprecation warnings) and because node.default broke tests.

The recipes I changed to node.default, which is a little different underneath because default attributes don’t persist and ones set by node.set/node.normal do. I did this for several reasons:

  • The Chef attributes doc recommends using node.default as often as possible.
  • The Chef Style Guide recommends avoiding node.set/node.normal.
  • The loop where they’re used executes on every Chef client run so it feels better to start with defaults and clean up only if needed, rather than preserving values from previous runs.

All ChefSpecs and kitchens pass.

There is already coverage of the legacy attr support for both recipes tests what gets rendered into the configs. I could add tests that the new attrs are updated when the old attrs are used, but that doesn't feel very valuable. I can add them if you think that’s better, though, no problem.

Let me know if you see any problems or you’d like anything done differently and I’ll rebase it in. Thanks!

Platform used to run ChefSpec:

> chef --version

Chef Development Kit Version: 0.17.17

chef-client version: 12.13.37

delivery version: master (f68e5c5804cd7d8a76c69b926fbb261e1070751b)

berks version: 4.3.5

kitchen version: 1.11.1

> chef exec rspec --version

3.5.2

operatingops added some commits Sep 24, 2016

Use node.normal instead of node.set in ChefSpec attributes.
node.set will be deprecated and using it generates warnings.
Use node.default instead of node.set in recipes.
node.set will be deprecated and using it generates warnings.
Although node.normal would have mirrored the original node.set,
the default level is sufficient and Chef recommends avoiding
node.normal.
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 24, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 3d7ff56 on operatingops:fix_deprecation_warnings into afff57b on dev-sec:master.

Coverage Status

Coverage remained the same at 100.0% when pulling 3d7ff56 on operatingops:fix_deprecation_warnings into afff57b on dev-sec:master.

@chris-rock

This comment has been minimized.

Show comment
Hide comment
@chris-rock

chris-rock Sep 25, 2016

Member

Awesome @operatingops Thanks for the update!

Member

chris-rock commented Sep 25, 2016

Awesome @operatingops Thanks for the update!

@chris-rock chris-rock merged commit 6b22038 into dev-sec:master Sep 25, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details
@chris-rock

This comment has been minimized.

Show comment
Hide comment
@chris-rock

chris-rock Sep 25, 2016

Member

I released a new version 1.2.1

Member

chris-rock commented Sep 25, 2016

I released a new version 1.2.1

@operatingops

This comment has been minimized.

Show comment
Hide comment
@operatingops

operatingops Sep 25, 2016

Contributor

No problem @chris-rock - thanks for the release!

Contributor

operatingops commented Sep 25, 2016

No problem @chris-rock - thanks for the release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment