Brice --
 
> > This patch modifies existing tests to pass with the new code (by
> >     stubbing out the additional FileTests) and adds a new test which
> >     catches the original problem.

> Why are your paragraphs indented like this?

I was attempting to follow the style guide Luke posted a link to, which
says "Use a hanging indent" though I notice in looking back at it that,
although the example text says to do this the example itself does not
use hanging indents.

> > +        return false unless FileTest.exist?(Puppet[:hostcert]) and  
> > FileTest.exist?(Puppet[:localcacert])

> Looks like there is a double-space or a tab in "and  FileTest".

You're right, it does.  My bad.

> > -        if Puppet[:http_enable_post_connection_check]
> > -            http.enable_post_connection_check = true
> > -        else
> > -            http.enable_post_connection_check = false
> > -        end
> > +        http.enable_post_connection_check = 
> > Puppet[:http_enable_post_connection_check]

> I'm really nitpicking here (I like to nitpick when it's not one of my
> patch, mouahahah :-)), but this is isn't strictly equivalent. 
> What if Puppet[:http_enable_post_connection_check] contains something
> that evaluates as being true but is not "True" (in the sense of the
> boolean object instance true).

> It might not matter of course, because this setting might always be
> strictly true or false, what matters is that we're giving this to
> someone else that might not have the same view on what's true or
> false...

Yeah, I agonized (briefly) over this one.  The code smell of the first
form is pretty intense, and my intent was to determine that it either
had to be this way, and document why, or determine that it didn't, and
fix it.

As it turns out, 1) existing uses will always set the source to either
true or false; 2) the result is only used in normal testing, as per ruby
standards (e.g. no stinky "if @enable_post_connection_check == true"
uses); it's always used exactly as the source value was being used in
the if statement here, so there is/was no reason for the convoluted
assignment.

-- Markus



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