On Wed, Jan 19, 2011 at 17:28, Bostjan Skufca
<[email protected]> wrote:

> After a brief discussion on puppet-users list I've completed the
> feature #5936 modification. The patch is below, feature ticket is
> opened and there in the comments there is also github pull url (can't
> edit it):

I followed up on the user list too, but my one comment is:

> +    globs.each do |glob|
> +      globDirs = Dir.glob(glob)

...so, this can match anything, not just directories. We need to
filter the output to exclude non-directory items, because otherwise
this is going to match someones editor file or fifo and cause us
grief.

I think the correct behaviour here would be:

globs.each do |pattern|
  globDirs = Dir.glob(pattern).select { |p| File.directory? p }.sort
  ...

The reason for sort at the end?  It gives predictable behaviour to end
users, which doesn't randomly change the order of things on their
module path when the directory sort order on disk changes during a
btree shuffle or something.

Otherwise this is a +1 from me.

Regards,
    Daniel
-- 
⎋ Puppet Labs Developer – http://puppetlabs.com
✉ Daniel Pittman <[email protected]>
✆ Contact me via gtalk, email, or phone: +1 (503) 893-2285
♲ Made with 100 percent post-consumer electrons

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