On 14/07/07, chromatic <[EMAIL PROTECTED]> wrote:
On Saturday 14 July 2007 09:05:02 [EMAIL PROTECTED] wrote:
> Author: paultcochrane
> Date: Sat Jul 14 09:05:01 2007
> New Revision: 19863
>
> Modified:
> trunk/lib/Parrot/Configure.pm
> trunk/lib/Parrot/Configure/Data.pm
> trunk/lib/Parrot/Configure/Step.pm
>
> Log:
> [lib] Using explicit returns as per coding standards
I don't trust some of these, and it makes me question the utility of this
coding standard test.
Comments very muchly welcome and duly noted. I'll back these changes
out. I'm still sometimes at the edge of my ability wrt perl so please
accept my apologies (I thought the return ... for did the right
thing).
Paul
> Modified: trunk/lib/Parrot/Configure/Data.pm
> ===========================================================================
>=== --- trunk/lib/Parrot/Configure/Data.pm (original)
> +++ trunk/lib/Parrot/Configure/Data.pm Sat Jul 14 09:05:01 2007
> @@ -210,7 +210,7 @@
> if ( not defined $res ) {
> die "You cannot use --step until you have completed the full
> configure process\n"; }
> - $self->{c}{$_} = $res->{$_}
> + return $self->{c}{$_} = $res->{$_}
> for CORE::keys %$res;
> }
This will process the first key (and why CORE::keys?) and then return. That
seems to me pretty clearly wrong, or at least a big change in behavior.
> Modified: trunk/lib/Parrot/Configure/Step.pm
> ===========================================================================
>=== --- trunk/lib/Parrot/Configure/Step.pm (original)
> +++ trunk/lib/Parrot/Configure/Step.pm Sat Jul 14 09:05:01 2007
> @@ -155,7 +155,7 @@
> sub move_if_diff {
> my ( $from, $to, $ignore_pattern ) = @_;
> copy_if_diff( $from, $to, $ignore_pattern );
> - unlink $from;
> + return unlink $from;
> }
I'm not sure the return value of unlink is useful here.
> =item C<genfile($source, $target, %options)>
> @@ -403,7 +403,7 @@
> close($in) or die "Can't close $source: $!";
> close($out) or die "Can't close $target: $!";
>
> - move_if_diff( "$target.tmp", $target, $options{ignore_pattern} );
> + return move_if_diff( "$target.tmp", $target, $options{ignore_pattern}
> ); }
>
> =item C<_run_command($command, $out, $err)>
> @@ -472,7 +472,7 @@
> sub cc_gen {
> my ($source) = @_;
>
> - genfile( $source, "test.c" );
> + return genfile( $source, "test.c" );
> }
>
> =item C<cc_build($cc_args, $link_args)>
> @@ -564,7 +564,7 @@
> =cut
>
> sub cc_clean {
> - unlink map "test$_", qw( .c .cco .ldo .out), $conf->data->get(qw( o
> exe )); + return unlink map "test$_", qw( .c .cco .ldo .out),
> $conf->data->get(qw( o exe )); }
>
> =item C<capture_output($command)>
I'm really not sure the return value from unlink is useful here.
