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

[Brian J. Miller]

Jon Jensen wrote:
> On Tue, 20 Jan 2009, Brian J. Miller wrote:
> 
>> 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.
> 
> To me, it's correct. One indentation level for line continuation, another 
> for the parentheses, then back one once the parens are done.
> 

+    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";

Isn't that two indents for line continuation? Why wouldn't the closing
paren line up under 'my'? Like:

+    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";

(Should also include the reason for failure in the error message)

>>> -    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.
> 
> Davor answered how it happens, and good suggestion. I've included $!.

Cool.

> 
>>> @@ -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?
> 
> Yes, I understood it's a functional change and checked for it.
> 

Okay...

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