On Thu Apr 24 20:49:05 2008, geoff wrote:
> On Thu, 2008-04-24 at 19:55 -0700, James Keenan via RT wrote:
> > Please see patch attached.  It turned out that config/auto/opengl.pm
> > didn't quite conform to the pattern, so I left it unchanged.
> 
> Other than having a Darwin case, how is it different than the pattern?
> 

The Darwin case is the difference.

> Also, the implementation of C<_add_to_libs> is a little wordy.  How's
> this?
> 
>  sub _add_to_libs {
>      my ($self, $args) = @_;
>      croak "_add_to_libs() takes hashref" unless ref($args) eq 'HASH';
> 
>      my $os       = $args->{osname};
>      my $cc       = $args->{cc};
>      my $platform = $os =~ /mswin32/i && $cc =~ /^gcc/i ? 'win32_gcc'   :
>                     $os =~ /mswin32/i                   ? 'win32_other' :
>                     $os =~ /darwin/i                    ? 'darwin'      :
>                                                         ? 'default'     ;
> 
>      my $libs     = $args->{$platform} || $args->{default};
> 
>      $args->{conf}->data->add(' ', libs => $libs);
>      return 1;
>  }
> 

Does this not imply that in the 4 classes addressed in the patch, the
user of this method would be required to supply a 'darwin' key-value
pair even though that the value for that key would be identical with
'non_win32' (or 'default')?

    $self->_add_to_libs( {
        conf        => $conf,
        osname      => $osname,
        cc          => $cc,
        win32_gcc   => '-lreadline',
        win32_other => 'readline.lib',
        darwin      => '-lreadline',
        default     => '-lreadline',
    } );


Do you think that the added generality of what is essentially an
internal helper method would then offset the additional complexity in
that method's interface?


Reply via email to