-
Notifications
You must be signed in to change notification settings - Fork 261
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
Enabled unified mode: CHEF-33 deprecation warning #822
Conversation
This mode will be the default on Chef 18, and it's defined as a current deprecation. See CHEF-33: https://docs.chef.io/deprecations_unified_mode/
As per the CHEF-33 deprecation, for Chef client versions lower than 14.14 the unified_mode is not enabled. Having in mind that even Chef 14 (and 15) are on EOL now, dropping support for Chef client 12 and 13 should not be an issue. See https://docs.chef.io/deprecations_unified_mode/
Related to #821 |
Thanks a lot for the patch! Do you think we could make this backwards compatible with Chef 12, 13 and <14.14? I was hoping we could avoid doing a new major version for this :/ |
Basing my answer on sous-chef/nginx cookbook PR to enable unified_mode, and reviewing the official documentation I think it’s even worse 😅! I think it’s required to drop support for Chef versions lower than 15.3! Regardless, maybe it’s possible to if/else in the custom resources depending on the Chef version? I haven’t seen that on other cookbooks but it may be worth the try. |
Yes, I think that should be fairly easy and would avoid having to do a major version bump 😃 |
This function should fix the CHEF-33 deprecation warning where maintaining backwards compatibility with Chef versions that can't enable it (15.0,15.1,15.2 and lower than 14.14). See https://docs.chef.io/deprecations_unified_mode/
This platforms are: - Chef 14.14+ but less than 15.0 - Chef 15.3+ See https://docs.chef.io/deprecations_unified_mode/
Hi @albertvaka ; after some smol vacations I've just uploaded some changes. Feel free to comment! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks a lot for updating the PR with the conditional unified_mode
: ) I left one minor comment but overall LGTM.
Co-authored-by: Albert Vaca Cintora <albertvaka@gmail.com>
The CI is failing but it seems to be due to rvm/rvm#5133 let me check if I can fix that... Edit: This PR should fix the CI #823 |
Set to true to treat deprecation warning messages as error messages. Useful on CI environments to prevent cookbook getting rusty and ease its maintenance.
This reverts commit a2c7ef3.
After merging your CI fix (3343988) it seems that the pipeline fails for old versions of Chef (14.10 and 13.12.14) like shown below. I'm not entirely sure this
Maybe someone with more knowledge on ruby than me could hack this? EDIT: I believe it may be working thanks to @tas50 chef wizardry (see job). |
This reverts commit 09a3861.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, this solution is even cleaner 🎉 I'll merge it now. Again, thanks a ton for investigating this and opening a PR!
We use this cookbook in our company, and by using in the
kitchen.yml
the setting ofdeprecations_as_errors
, a CHEF-33 deprecation warning came up, so this should fix it by:Dropping support for unsupported Chef client versions that are already on EOL (12, 13 and <14.14).I think this should be a major change (so v5.0.0) so I took the time to modify the CHANGELOG as well, but feel free to modify this pull request!Cheers