On 27/03/10 03:10, Markus Roberts wrote:
>> Which previously was:
>
>> source = base || event.source
>>
>>> leads to odd transformations when translated to the present code (e.g.
>> right
>>> after your branch is merged into testing):
>>>
>>> diff --git a/lib/puppet/simple_graph.rb b/lib/puppet/simple_graph.rb
>>> index 37a08e7..9c04a32 100644
>>> --- a/lib/puppet/simple_graph.rb
>>> +++ b/lib/puppet/simple_graph.rb
>>> @@ -158,11 +158,11 @@ class Puppet::SimpleGraph
>>> if base
>>> # consider only edges which are not pointing to any event
>>> sources
>>> # which is what a recursive file resources produces
>>> - event_sources = events.inject({}) { |hash, event|
>>> hash[event.source] = event.source ; hash}
>>> - edges = (adjacent(base, :direction => :out, :type => :edges)
>> -
>>> event_sources.keys)
>>> + event_sources = {base => base}
>>> + edges = (adjacent(base, :direction => :out, :type => :edges)
>> -
>>> [base])
>>> else
>>
>> This compares apples to oranges: the idea is to get the list of all
>> incoming event source (resource strings now), then to remove them from
>> the list of adjacent vertices (resources instances).
>>
>> The issue, assuming event.resource contains a resource ref string (I'm
>> still not sure about this), is to find the correct resource instances
>> (ie vertices in the graph) so that we can exclude them from the adjacent
>> vertices.
>>
>>> In the if-base clause, source would always == base, and walking that fact
>>> through leads to code that I suspect must be wrong.
>>
>> No the code is correct, it is just not equivalent to what it was
>> previously.
>>
>
> So (if it is, as you say, correct) why don't we just write it that way:
I meant my previous code is correct, not the one you provided.
> event_sources = {base => base}
> edges = (adjacent(base, :direction => :out, :type => :edges) -
> [base])
>
> and be done with it in that case? Why loop through the evens adding base =>
> base to the hash?
As I said earlier your proposed code isn't equivalent to the version I
did for 0.25.x.
Compare with the 0.25.x version:
event_sources = events.inject({}) { |hash, event| hash[event.source] =
event.source ; hash}
edges = (adjacent(base, :direction => :out, :type => :edges) -
event_sources.keys)
We collect all the distinct event.source from the given events, which we
then remove from all the adjacent vertices of base.
Your code just remove base from the adjacent vertices of base which is
not good (and not the same).
> P.S. As to indentation, etc.: the HEAD of the testing branch is what we're
> wanting the code to look like from Rowlf onward; until then, everything
> should still look as it has until now. As Luke mentioned, this is part of
> our effort to make our ruby look like other people's ruby.
Yes, I got it :-)
I will do a new #3396 for testing (based on Luke's events branch or
testing earlier in the commit history) this week-end (formatted with 4
spaces of course :-)).
--
Brice Figureau
My Blog: http://www.masterzen.fr/
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
