Re: invalid installdirs values patch

On Sep 27, 2008, at 21:59, Eric Wilhelm wrote:

>>> Could we have that with the original indentation please?
>>
>> Which is what? I can't make heads or tails of it.
>
> Ack.  I should have said "without the 32-line ternary".  Please.  I'm
> all for refactoring, but some of that should really come in more than
> one piece.

Ah, so you *were* able to read some of the patch, eh? ;-) I'm fine to  
change the ternary, it's a dead simple change.

> Otherwise it is very difficult to review.

I somehow doubt that the ternary made it difficult to review. When I  
change it, it will be very nearly identical.

> I think this needs to seek "one of the other ways to do it":
>  # Allow to work in class or object context.
>  my $x = do { no strict 'refs'; $self->{properties} };
>  return $x->{$property} unless @_;

Oh, big time. But that was how I got the tests to pass. I have no idea  
why properties need to work in a class context.

> When you have two 'it's, one of them should have a name
>    for (reverse shift->_mb_classes) {
>      @out{ keys %{ $valid_properties{$_} } } = map {
>          $_->()
>      } values %{ $valid_properties{$_} };
>    }

Every used a Schwartzian transform?

> But, I have doubts about the "everything is a sub" approach here.  If
> the default is a subref, that implies that we always call it (e.g.
> creating a new anonymous ref ala moose) rather than ever holding the
> value -- and if that were the case then valid_properties_defaults()
> would not exist?  That is, take a look at _set_defaults().

I don't follow you here. What I wanted was an easy way for a class to  
define a default at runtime. It's just a simple thing; if people hate  
it, I can remove it.

Best,

David