On May 23, 2011, at 11:45 AM, Dan Bode wrote:

> This is an experimental set of patches that I wrote for composite namevars 
> to work with parsedfile and to support purging.
> 
> This is not ready to be merged, I would just like to get some input on the 
> following:
>  - does this work (for people who have pending composite namevar work)
>  - will this break anything? although it has passed the unit test,
>    it touches Puppet in scary places that I mostly, but do not fully 
>    understand
>  - is the design agreeable enough for everyone?

One of the major concerns I have is that there aren't any tests for any of it.

It's a very complicatd area of functionality, and not having clarity on desired 
behavior in the form of tests makes it much harder to maintain over time.

Also, the majority of these patches, possibly all of them, should be squashed 
into one patch.  The only one that might deserve separate treatment is the 
parsedfile patch, but I'm not even convinced about that one.  With the spread 
of patches, it's kind of hard to see what the actual design is, and it's 
especially hard, regardless of the number of patches, to see what behavior we 
had and what this changes it to.

Can you provide a summary of that somewhere in one of the commits?

And finally, the commit subjects are a bit long, and, um, weirdly capitalized.  
It'd be nice to see that cleaner.

-- 
A computer lets you make more mistakes faster than any invention in
human history--with the possible exceptions of handguns and tequila.
                -- Mitch Ratcliffe
---------------------------------------------------------------------
Luke Kanies  -|-   http://puppetlabs.com   -|-   http://about.me/lak




-- 
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.

Reply via email to