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

CHEF-4553: DSL support for customizable guards for powershell_script, bash, et. al resource scenarios #5

Merged
merged 24 commits into from
Jun 29, 2014

Conversation

adamedx
Copy link

@adamedx adamedx commented Feb 7, 2014

I'm still updating this, but I think there's enough here to comment.

One thing I don't like: not sure the name "anonymous resource" is what I'm really looking for here -- suggestions welcome!

@dawilliams
Copy link

+1 This would really help me keep my guards for the powershell_script resource easier and cleaner.

environment variable, alternate user identity, or current directory, and
testing the current state to ensure idempotence through a guard requires the
same state. Inheritance enables that state to be expressed exactly once
through the Chef DSL.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to make it inherit more narrowly, only when the parent resource is the same class as the guard resource? Or are you purposefully allowing for different classes of execute resources to inherit cwd? I can see the utility in duck typing this way, but I can also see a possible edge case where the name of an attribute in the parent might be the same but the parent might be some strange LWRP with different semantics and this causes surprise.

Copy link
Author

Choose a reason for hiding this comment

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

Very good point. At the very least, it should probably be limited to cases where the class in the guard expression is a derived class of the parent. Then there can't be a conflict, since those attributes must already be defined by the parent.

@juliandunn
Copy link
Contributor

@adamedx is there more to do before merging this? the implementation's done now, right?

@adamedx
Copy link
Author

adamedx commented Jun 4, 2014

Yes, but the doc hasn't been updated, so I'll do a merge after I finish the updates that take into account the final feedback (i.e. what was actually implemented).

adamedx pushed a commit that referenced this pull request Jun 29, 2014
CHEF-4553: DSL support for customizable guards for powershell_script, bash, et. al resource scenarios
@adamedx adamedx merged commit 32c8bef into master Jun 29, 2014
@adamedx adamedx deleted the adamed/resource-guard branch June 29, 2014 03:48
@juliandunn juliandunn mentioned this pull request Jul 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants