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?