[Camps-users] [Camps-commits] [SCM] Camps development environments branch, master, updated. 71dc7b5e3651237c99f116565bac75474d378a5e

[Brian J. Miller]

Jon Jensen wrote:
> This is an automated email from the git hooks/post-receive script. It was
> generated because a ref change was pushed to the repository containing
> the project "Camps development environments".
> 
> The branch, master has been updated
>        via  71dc7b5e3651237c99f116565bac75474d378a5e (commit)
>       from  eca67cf96aac67f2e86f42e83e4327eb55fb75de (commit)
> 
> Those revisions listed above that are new to this repository have
> not appeared on any other notification email; so we list those
> revisions in full, below.
> 
> - Log -----------------------------------------------------------------
> commit 71dc7b5e3651237c99f116565bac75474d378a5e
> Author: Jon Jensen <jon at endpoint.com>
> Date:   Wed Jan 14 18:29:38 2009 -0700
> 
>     Code cleanup based on perlcritic run and some whitespace & formatting standardization.
> 
> -----------------------------------------------------------------------
> 
> Summary of changes:
>  lib/Camp/Config.pm |  153 +++++++++++++++------------------
>  lib/Camp/Master.pm |  240 ++++++++++++++++++++++-----------------------------
>  2 files changed, 172 insertions(+), 221 deletions(-)
> 
> diff --git a/lib/Camp/Config.pm b/lib/Camp/Config.pm
> index 32fa49d..41435c5 100644

[snip]

>  
> @@ -357,11 +350,12 @@ sub dbh {
>          }
>          unless ref $options eq 'HASH';
>  
> -    my $dbh = DBI->connect($invocant->db_dsn($catalog),
> -               $invocant->db_user($catalog),
> -               $invocant->db_password($catalog),
> -               $options)
> -    or die "Unable to obtain database handle";
> +    my $dbh = DBI->connect(
> +            $invocant->db_dsn($catalog),
> +            $invocant->db_user($catalog),
> +            $invocant->db_password($catalog),
> +            $options
> +        ) or die "Unable to obtain database handle\n";
>  

The above seems like incorrect indentation. Honestly I don't care that
much, but if this is generated by Perl::Critic I thought it best to
point it out.

>      return $dbh;
>  }

[snip]

> @@ -536,24 +524,24 @@ sub _parse_file {
>          ) {
>              my $variable = $1;
>              $val =~ s/\s+$// if defined $val;
> -            $val = $invocant->_substitute_variables( $catalog, $val );
> -            $invocant->_variable_set( $catalog, $variable, $val );
> +            $val = $invocant->_substitute_variables($catalog, $val);
> +            $invocant->_variable_set($catalog, $variable, $val);
>          }
>          elsif (
>              $key eq 'include'
>              and my @incfiles = grep -f $_, glob($val)
>          ) {
>              for (@incfiles) {
> -                $invocant->_parse_file( $catalog, $_ );
> +                $invocant->_parse_file($catalog, $_);
>              }
>          }
> -        
> +
>          $mark_type = $key = $val = $marker = undef;
>      }
> -    
> +
>      die "Configuration file '$file' contains unterminated <<HERE doc (<$mark_type$marker)!\n" if defined $marker;
>      die "Configuration file '$file' contained unterminated #ifdef!\n" if defined $ifdef;
> -    close $CONF;
> +    close $CONF or die "Error closing $file\n";
>  

I've never been real clear on why this would happen, should this be an
exception over a warning? (There are a couple of additional ones like
this that I'm going to snip out below.) Either way this exception should
include $! so that we know why, arguably which file couldn't be closed
is less valuable.

[snip]

> diff --git a/lib/Camp/Master.pm b/lib/Camp/Master.pm
> index 6b70d29..06205e2 100644
> --- a/lib/Camp/Master.pm
> +++ b/lib/Camp/Master.pm

[snip]

>  
> @@ -615,6 +620,7 @@ sub set_camp_user {
>      ;
>      $camp_user = $camp_user_obj->name;
>      $camp_user_info = set_camp_user_info( $camp_user );
> +    return;

This is a functional change, and a possibly significant one. But I have
no idea whether it matters, was it tested, or the calling places checked?

>  }
>  
>  sub set_camp_user_info {

[snip]

> @@ -750,6 +755,8 @@ sub _config_hash_db {
>      else {
>          die "Unknown database type!\n";
>      }
> +
> +    return;

C<return> here similar to up above, though I'm less concerned about that.

>  }
>  
>  sub _determine_db_type_and_path {

[snip]

> @@ -1372,6 +1373,7 @@ sub create_camp_path {
>      my $cfg = config_hash( $camp );
>      cleanup_camp_path($camp, $replace);
>      mkpath($cfg->{path}) or die "Couldn't make camp path: $cfg->{path}: $!\n";
> +    return;

Same with return.

>  }
>  
>  sub register_camp {

[snip]

> @@ -1445,8 +1447,8 @@ sub git_repository {
>              $hash->{repo_path_git},
>              $hash->{repo_path},
>              'repo.git',
> -        )
> -    ;
> +        );
> +    return;

Same. And I'm going to now stop including these.

>  }
>  
>  sub svn_repository {

[snip]

-- 
Brian J. Miller
End Point Corp.
brian at endpoint.com
W: 704-376-8292