Re: [PATCH] git-send-email.perl - Fix 550 EHLO argument does not match calling host

[Jakub Narebski <jnareb@gmail.com>]

Jari Aalto <jari.aalto@cante.net> writes:

> +sub maildomain_net
> +{
> +	my $maildomain;
> +	eval "use Net::Domain";
> +
> +	unless ($@) {

I think the Perl idiom for this is

   	if (eval { require Net::Domain; 1 }) {

Note the block form of eval (which is more efficient, and is usual way
of trap exceptions in Perl), using return value of eval instead of
using $@ variable, and using 'require' rather than 'use'.

> +		eval "use Net::Domain";
> +		unless ($@) {

Why this duplication?  You have 'eval unless eval unless', with 
'eval unless' twice.

> +		    my $domain = Net::Domain::domainname();
> +		    $maildomain = $domain
> +			    unless $^O eq 'darwin' && $domain =~ /\.local$/;

I'd like to have a comment about the above line: why it is necessary?

> +		}
> +	}
> +
> +	$maildomain;

It is a matter of style, but unless function is a very simple one, the
preferred way is to use explicit return statement, i.e.

   	return $maildomain;

> +}

-- 
Jakub Narebski
Poland
ShadeHawk on #git