Avoid redefining the Chef::Resource#name method #116
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In chef/chef#5606, Chef will now log deprecation warnings whenever a resource defines a property that is already an instance_method on that resource. If this happens, a user may be inadvertently causing unexpected and difficult-to-troubleshoot behavior. A number of the cheffish resources were defining a `name` property which is already a property defined on the Chef::Resource base class. This change stops defining the `name` property for each of these resources and instead defines something unique. In addition, the `chef_mirror` resource defined a `freeze` property which is already an instance_method on `Object` instances. I have changed that to be `freeze_on_upload` and defined a method to log a deprecation warning while providing backward compatibility. Signed-off-by: Adam Leff <[email protected]>
adamleff
force-pushed
the
adamleff/stop_using_name_property
branch
from
31bf35c
to
c7d5174
Compare
|
👍 although now that i see this, i'm suspecting that other users out there may have overloaded the name property to introduce validations. this is still the right thing to do here and should be merged, but i'm not sure the chef/chef PR should be merged... |
thommay
approved these changes
Dec 6, 2016
thommay
added
Improvement
Type: Enhancement
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In chef/chef#5606, Chef will now log deprecation warnings whenever
a resource defines a property that is already an instance_method
on that resource. If this happens, a user may be inadvertently
causing unexpected and difficult-to-troubleshoot behavior.
A number of the cheffish resources were defining a
namepropertywhich is already a property defined on the Chef::Resource base class.
This change stops defining the
nameproperty for each of theseresources and instead defines something unique.
In addition, the
chef_mirrorresource defined afreezepropertywhich is already an instance_method on
Objectinstances. I havechanged that to be
freeze_on_uploadand defined a method to loga deprecation warning while providing backward compatibility.