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