Re: [PATCH v2 3/3] send-email: Add sendemail.smtpdomain

From: Brian Gernhardt <brian@gernhardtsoftware.com>
Date: Apr 10, 2010, 13:51 UTC
Message-ID: <503EF023-77C0-4763-947C-385C67D3875A@gernhardtsoftware.com>
In-Reply-To: <201004092040.50142.jnareb@gmail.com>

On Apr 9, 2010, at 2:40 PM, Jakub Narebski wrote:

Show 17 quoted lines

> On Fri, 9 Apr 2010, Brian Gernhardt wrote:
> 
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index f491d44..bdfe3f2 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -132,8 +132,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };
>> my $have_mail_address = eval { require Mail::Address; 1 };
>> my $smtp;
>> my $auth;
>> -my $mail_domain_default = "localhost.localdomain";
>> -my $mail_domain;
>> +my $smtp_domain_default = "localhost.localdomain";
> 
> Why this change, this renaming of variables from $mail_domain_default
> and $mail_domain to $smtp_domain_default and $smtp_domain?  Why you
> have removed this forward declaration of $smtp_domain/$mail_domain?

I changed it to better match many of the other variables in the script.  The variables that hold command line options are usually named after the option.  I left the default there as this seemed to be an area of the script that set "constant" variables (like $have_email_valid and $have_mail_address).

Show 14 quoted lines

>> sub unique_email_list(@);
>> sub cleanup_compose_files();
>> @@ -190,7 +189,7 @@ sub do_edit {
>> # Variables with corresponding config settings
>> my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);
>> my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);
>> -my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);
>> +my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts, $smtp_domain);
>> my ($validate, $confirm);
>> my (@suppress_cc);
> 
> Why have you moved $smtp_domain declaration (formerly $mail_domain)
> here?  And why it is not described in commit message (at least "Cleanup.",
> or something like this)?

$smtp_domain moved here to match comment just above "Variables with corresponding config settings".  $mail_domain was up near some unrelated globals and I wanted to move it near other similar variables.

Yes, all of this should have been in my commit message.  I blame the hour I was originally coding.  I had intended to fix my Python problem and send it in, but then I had to fix send-email because my MTA complained about "EHLO My-Computer".

I appreciate the review and help, although I wish I wasn't making a fool of myself in public.  ;-)

~~ Brian Gernhardt