Re: [PATCH v2 1/3] send-email: Don't use FQDNs without a '.'

From: Brian Gernhardt <brian@gernhardtsoftware.com>
Date: Apr 10, 2010, 13:44 UTC
Message-ID: <CD79B09D-D728-4A3A-945F-9D117C15FFB5@gernhardtsoftware.com>
In-Reply-To: <201004091831.49066.jnareb@gmail.com>

On Apr 9, 2010, at 12:31 PM, Jakub Narebski wrote:

> Note that the comments below are just nitpicking about Perl style.

Fair enough.  I've been using Ruby and Shell far more than Perl recently.  I've gotten a bit rusty.

Show 16 quoted lines

> A matter of style: in Perl it more usual to use
> 
>  sub <name> {
>  	...
>  }
> 
> style rather than
> 
>  sub <name>
>  {
>  	...
>  }
> 
> Unfortunately git-send-email.perl is a bit inconsistent in the style used;
> 23 subroutines use Perl style, 5 subroutines including previous one i.e.
> sanitize_address use C-like style (one of).

I was copying style from the other functions I was working on.  I'll make my additions more "standard" and add a patch to clean up the rest.

Show 10 quoted lines

> Also, the usual way of unrolling @_; is to use either
> 
>  my ($par1, $par2, ...) = @_;
> 
> or use
> 
>  mu $par = shift;
> 
> The form $_[0] etc. is used very rarely.  I think it is even against 
> Perl Best Practices (see http://www.perlcritic.org and Perl::Critic).

I knew that.  I really did.  But I started off trying to write

sub valid_fqdn( $domain )

Which would be valid Perl 6, but not Perl 5.  So then I tried using

my $domain = $1

Which, while valid, is wrong.  So I changed it to @_[1], @_[0], and finally $_[0].  My brain wasn't running at 100% yesterday, apparently.

> Style: usually there is no space around function arguments, so 
> 'valid_fqdn($domain);'.

University training is difficult to overcome.  They demanded spaces nearly everywhere, so I type them by something akin to reflex.

Thank you for all the review! ~~ Brian Gernhardt