Skip to content
This repository has been archived by the owner on Sep 19, 2020. It is now read-only.

resource_attributes should show all of the relevant part of the AST #31

Closed
jonlives opened this issue May 15, 2012 · 7 comments
Closed
Labels

Comments

@jonlives
Copy link

In the event of a resource which contains a notifies, for example, resource_attributes shows the following:

[7] pry(#FoodCritic::RuleDsl)> resource_attributes(cmd)
=> {:name=>"/etc/nscd.conf",
"backup"=>"",
"source"=>"nscd.conf",
"owner"=>"root",
"group"=>"root",
"mode"=>"",
"notifies"=>:restart}

Ideally we'd be able to see what the "notifies" is being sent to, in this specific case shown in the <method_add_arg> tags in the AST.

@acrmp
Copy link
Member

acrmp commented May 17, 2012

Hi Jon,

Thanks for reporting this one. By all accounts the Etsy session yesterday rocked.

I'm suffering the effects of a cold at the moment but will look into this on the weekend.

Cheers,

Andrew.

acrmp pushed a commit that referenced this issue May 21, 2012
@acrmp
Copy link
Member

acrmp commented May 21, 2012

Hi Jon,

This change has been released in 1.3.0. Please let me know if you can now see the AST as expected from your rules.

Cheers,

Andrew.

@jonlives
Copy link
Author

Andrew,

I can indeed get the AST as expected from the rule, however the AST returned doesn't seem to work with resource_attribute or resource_attributes (which returns a blank hash).

This is the AST I'm getting: https://gist.github.com/2780473 based on running resource_attribute(cmd, 'notifies') on the AST. When I try and run resource_attribute or resource_attributes on this resulting AST, the return value is blank.

I have a sneaky suspicion that those methods may not be expected to work on this sub-AST, but it seems like they should, otherwise I just have a slightly smaller AST to write an xpath against :p

Cheers,

Jon

@acrmp
Copy link
Member

acrmp commented May 24, 2012

Hi Jon,

Yes, the resulting AST isn't supposed to be able to be used as input to resource_attributes. You are only getting more convenient access to that part of the tree.

It looks like you need another function to extract simple expressions from the hash?

Cheers,

Andrew.
(from my phone)

@jonlives
Copy link
Author

Ah yeah I thought as much - a function to extract stuff from that hash would be awesome tho, my main motivation for logging this issue was to avoid writing xpaths :p

acrmp pushed a commit that referenced this issue Jun 5, 2012
acrmp pushed a commit that referenced this issue Jun 6, 2012
acrmp pushed a commit that referenced this issue Jun 6, 2012
acrmp pushed a commit that referenced this issue Jun 6, 2012
Avoids users of #notifications having to switch on both.
@acrmp
Copy link
Member

acrmp commented Jun 6, 2012

Hi Jon,

I've added a #notifications method to the API which you can use to get a more usable hash with the notification details.

Here's an example making use of it.

Cheers,

Andrew.

@jonlives
Copy link
Author

jonlives commented Jun 7, 2012

Awesome, that's exactly what I was looking for. I'll close this one off :)

@jonlives jonlives closed this as completed Jun 7, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants