{"thread":{"id":"23392","subject":"[PATCH v2 0/3] *** SUBJECT HERE ***","startedAt":"2010-04-09T15:34:02Z","lastAt":"2010-04-10T13:51:36Z","messageCount":12,"participants":["Brian Gernhardt","Sverre Rabbelier","Jakub Narebski"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"139043","messageId":"1270827245-28958-1-git-send-email-brian@gernhardtsoftware.com","threadId":"23392","inReplyTo":null,"subject":"[PATCH v2 0/3] *** SUBJECT HERE ***","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-04-09T15:34:02Z","receivedAt":"2010-04-09T15:34:02Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"Changes from v1:\n - Moves valid FQDN conditions from line-ending conditions to a new\n function\n - Adds sendemail.smtpdomain to the list in config.txt\n\nPatch 2/3 is unchanged.\n\nBrian Gernhardt (3):\n send-email: Don't use FQDNs without a '.'\n Document send-email --smtp-domain\n send-email: Add sendemail.smtpdomain\n\n Documentation/config.txt | 1 +\n Documentation/git-send-email.txt | 7 +++++++\n git-send-email.perl | 32 ++++++++++++++++++--------------\n 3 files changed, 26 insertions(+), 14 deletions(-)\n"},{"id":"139047","messageId":"1270827245-28958-2-git-send-email-brian@gernhardtsoftware.com","threadId":"23392","inReplyTo":"1270827245-28958-1-git-send-email-brian@gernhardtsoftware.com","subject":"[PATCH] Makefile: Simplify handling of python scripts","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-04-09T15:34:03Z","receivedAt":"2010-04-09T15:34:03Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"The sed script intended to add a standard opening to python scripts\nwas non-compatible and overly complex. Simplifying it down to a set\nof one-liners removes the compatibility issues of newlines. Moving\nthe environment alterations from the Makefile to the python scripts\nmakes also makes the scripts easier to run in-place.\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Brian Gernhardt \n---\n Makefile | 9 ++-------\n git-remote-testgit.py | 2 ++\n 2 files changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex f0fe351..87c90d6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1629,13 +1629,8 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py\n \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \\\n \t\t--no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' \\\n \t\tinstlibdir` && \\\n-\tsed -e '1{' \\\n-\t -e '\ts|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n-\t -e '}' \\\n-\t -e 's|^import sys.*|&; \\\\\\\n-\t import os; \\\\\\\n-\t sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\\\n-\t \"@@INSTLIBDIR@@\"));|' \\\n+\tsed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n+\t -e 's|\\(os\\.getenv(\"GITPYTHONLIB\"\\)[^)]*)|\\1,\"@@INSTLIBDIR@@\")|' \\\n \t -e 's|@@INSTLIBDIR@@|'\"$$INSTLIBDIR\"'|g' \\\n \t $@.py >$@+ && \\\n \tchmod +x $@+ && \\\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex f61624e..9253922 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -2,6 +2,8 @@\n \n import hashlib\n import sys\n+import os\n+sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\".\"))\n \n from git_remote_helpers.util import die, debug, warn\n from git_remote_helpers.git.repo import GitRepo\n-- \n1.7.1.rc0.243.g2ce66\n"},{"id":"139045","messageId":"1270827245-28958-3-git-send-email-brian@gernhardtsoftware.com","threadId":"23392","inReplyTo":"1270827245-28958-1-git-send-email-brian@gernhardtsoftware.com","subject":"[PATCH v2 2/3] Document send-email --smtp-domain","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-04-09T15:34:04Z","receivedAt":"2010-04-09T15:34:04Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"Signed-off-by: Brian Gernhardt \n---\n Documentation/git-send-email.txt | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex ced35b2..f171471 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -119,6 +119,12 @@ Sending\n \tvalue reverts to plain SMTP. Default is the value of\n \t'sendemail.smtpencryption'.\n \n+--smtp-domain=::\n+\tSpecifies the Fully Qualified Domain Name (FQDN) used in the\n+\tHELO/EHLO command to the SMTP server. Some servers require the\n+\tFQDN to match your IP address. If not set, git send-email attempts\n+\tto determine your FQDN automatically.\n+\n --smtp-pass[=]::\n \tPassword for SMTP-AUTH. The argument is optional: If no\n \targument is specified, then the empty string is used as\n-- \n1.7.1.rc0.243.g2ce66\n"},{"id":"139044","messageId":"1270827245-28958-4-git-send-email-brian@gernhardtsoftware.com","threadId":"23392","inReplyTo":"1270827245-28958-1-git-send-email-brian@gernhardtsoftware.com","subject":"[PATCH v2 3/3] send-email: Add sendemail.smtpdomain","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-04-09T15:34:05Z","receivedAt":"2010-04-09T15:34:05Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"--smtp-domain is an option that if you need once, you probably will\nneed again. To help with that, allow the user to set it in their\n.gitconfig\n\nSigned-off-by: Brian Gernhardt \n---\n Documentation/config.txt | 1 +\n Documentation/git-send-email.txt | 3 ++-\n git-send-email.perl | 20 ++++++++++----------\n 3 files changed, 13 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 626b19a..b755beb 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1651,6 +1651,7 @@ sendemail.smtppass::\n sendemail.suppresscc::\n sendemail.suppressfrom::\n sendemail.to::\n+sendemail.smtpdomain::\n sendemail.smtpserver::\n sendemail.smtpserverport::\n sendemail.smtpuser::\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex f171471..288a4ec 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -123,7 +123,8 @@ Sending\n \tSpecifies the Fully Qualified Domain Name (FQDN) used in the\n \tHELO/EHLO command to the SMTP server. Some servers require the\n \tFQDN to match your IP address. If not set, git send-email attempts\n-\tto determine your FQDN automatically.\n+\tto determine your FQDN automatically. Default is the value of\n+\t'sendemail.smtpdomain'.\n \n --smtp-pass[=]::\n \tPassword for SMTP-AUTH. The argument is optional: If no\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex f491d44..bdfe3f2 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -132,8 +132,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };\n my $have_mail_address = eval { require Mail::Address; 1 };\n my $smtp;\n my $auth;\n-my $mail_domain_default = \"localhost.localdomain\";\n-my $mail_domain;\n+my $smtp_domain_default = \"localhost.localdomain\";\n \n sub unique_email_list(@);\n sub cleanup_compose_files();\n@@ -190,7 +189,7 @@ sub do_edit {\n # Variables with corresponding config settings\n my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);\n my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);\n-my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);\n+my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts, $smtp_domain);\n my ($validate, $confirm);\n my (@suppress_cc);\n \n@@ -212,6 +211,7 @@ my %config_settings = (\n \"smtpserverport\" => \\$smtp_server_port,\n \"smtpuser\" => \\$smtp_authuser,\n \"smtppass\" => \\$smtp_authpass,\n+\t\"smtpdomain\" => \\$smtp_domain,\n \"to\" => \\@to,\n \"cc\" => \\@initial_cc,\n \"cccmd\" => \\$cc_cmd,\n@@ -283,7 +283,7 @@ my $rc = GetOptions(\"sender|from=s\" => \\$sender,\n \t\t \"smtp-ssl\" => sub { $smtp_encryption = 'ssl' },\n \t\t \"smtp-encryption=s\" => \\$smtp_encryption,\n \t\t \"smtp-debug:i\" => \\$debug_net_smtp,\n-\t\t \"smtp-domain:s\" => \\$mail_domain,\n+\t\t \"smtp-domain:s\" => \\$smtp_domain,\n \t\t \"identity=s\" => \\$identity,\n \t\t \"annotate\" => \\$annotate,\n \t\t \"compose\" => \\$compose,\n@@ -904,7 +904,7 @@ sub maildomain_mta\n \n sub maildomain\n {\n-\treturn maildomain_net() || maildomain_mta() || $mail_domain_default;\n+\treturn maildomain_net() || maildomain_mta() || $smtp_domain_default;\n }\n \n # Returns 1 if the message was sent, and 0 otherwise.\n@@ -1009,18 +1009,18 @@ X-Mailer: git-send-email $gitversion\n \t\tif ($smtp_encryption eq 'ssl') {\n \t\t\t$smtp_server_port ||= 465; # ssmtp\n \t\t\trequire Net::SMTP::SSL;\n-\t\t\t$mail_domain ||= maildomain();\n+\t\t\t$smtp_domain ||= maildomain();\n \t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n-\t\t\t\t\t\t Hello => $mail_domain,\n+\t\t\t\t\t\t Hello => $smtp_domain,\n \t\t\t\t\t\t Port => $smtp_server_port);\n \t\t}\n \t\telse {\n \t\t\trequire Net::SMTP;\n-\t\t\t$mail_domain ||= maildomain();\n+\t\t\t$smtp_domain ||= maildomain();\n \t\t\t$smtp ||= Net::SMTP->new((defined $smtp_server_port)\n \t\t\t\t\t\t ? \"$smtp_server:$smtp_server_port\"\n \t\t\t\t\t\t : $smtp_server,\n-\t\t\t\t\t\t Hello => $mail_domain,\n+\t\t\t\t\t\t Hello => $smtp_domain,\n \t\t\t\t\t\t Debug => $debug_net_smtp);\n \t\t\tif ($smtp_encryption eq 'tls' && $smtp) {\n \t\t\t\trequire Net::SMTP::SSL;\n@@ -1043,7 +1043,7 @@ X-Mailer: git-send-email $gitversion\n \t\t\tdie \"Unable to initialize SMTP properly. Check config and use --smtp-debug. \",\n \t\t\t \"VALUES: server=$smtp_server \",\n \t\t\t \"encryption=$smtp_encryption \",\n-\t\t\t \"maildomain=$mail_domain\",\n+\t\t\t \"maildomain=$smtp_domain\",\n \t\t\t defined $smtp_server_port ? \"port=$smtp_server_port\" : \"\";\n \t\t}\n \n-- \n1.7.1.rc0.243.g2ce66\n"},{"id":"139049","messageId":"i2ifabb9a1e1004090839hd55a81bfid820180815c8d94d@mail.gmail.com","threadId":"23392","inReplyTo":"1270827245-28958-2-git-send-email-brian@gernhardtsoftware.com","subject":"Re: [PATCH] Makefile: Simplify handling of python scripts","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-04-09T15:39:12Z","receivedAt":"2010-04-09T15:39:12Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Apr 9, 2010 at 17:34, Brian Gernhardt\n wrote:\n> The sed script intended to add a standard opening to python scripts\n> was non-compatible and overly complex.  Simplifying it down to a set\n> of one-liners removes the compatibility issues of newlines.  Moving\n> the environment alterations from the Makefile to the python scripts\n> makes also makes the scripts easier to run in-place.\n\nMy sed foo is not that great, can you explain (in the commit message)\nwhat exactly the new sed script does?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"139050","messageId":"1270827746-29229-1-git-send-email-brian@gernhardtsoftware.com","threadId":"23392","inReplyTo":"1270827245-28958-1-git-send-email-brian@gernhardtsoftware.com","subject":"[PATCH v2 1/3] send-email: Don't use FQDNs without a '.'","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-04-09T15:42:26Z","receivedAt":"2010-04-09T15:42:26Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"Although Net::Domain::domainname attempts to be very thorough, the\nhost's configuration can still refuse to give a FQDN. Check to see if\nwhat we receive contains a dot as a basic sanity check.\n\nSince the same condition is used twice and getting complex, let's move\nit to a new function.\n\nSigned-off-by: Brian Gernhardt \n---\n git-send-email.perl | 12 ++++++++----\n 1 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex ce569a9..f491d44 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -863,14 +863,19 @@ sub sanitize_address\n # This maildomain*() code is based on ideas in Perl library Test::Reporter\n # /usr/share/perl5/Test/Reporter/Mail/Util.pm ==> sub _maildomain ()\n \n+sub valid_fqdn\n+{\n+\tmy $domain = $_[0];\n+\treturn !($^O eq 'darwin' && $domain =~ /\\.local$/) && $domain =~ /\\./;\n+}\n+\n sub maildomain_net\n {\n \tmy $maildomain;\n \n \tif (eval { require Net::Domain; 1 }) {\n \t\tmy $domain = Net::Domain::domainname();\n-\t\t$maildomain = $domain\n-\t\t\tunless $^O eq 'darwin' && $domain =~ /\\.local$/;\n+\t\t$maildomain = $domain if valid_fqdn( $domain );\n \t}\n \n \treturn $maildomain;\n@@ -887,8 +892,7 @@ sub maildomain_mta\n \t\t\t\tmy $domain = $smtp->domain;\n \t\t\t\t$smtp->quit;\n \n-\t\t\t\t$maildomain = $domain\n-\t\t\t\t\tunless $^O eq 'darwin' && $domain =~ /\\.local$/;\n+\t\t\t\t$maildomain = $domain if valid_fqdn( $domain );\n \n \t\t\t\tlast if $maildomain;\n \t\t\t}\n-- \n1.7.1.rc0.243.g2ce66\n"},{"id":"139051","messageId":"1270828665-29373-1-git-send-email-brian@gernhardtsoftware.com","threadId":"23392","inReplyTo":"i2ifabb9a1e1004090839hd55a81bfid820180815c8d94d@mail.gmail.com","subject":"[PATCH v2] Makefile: Simplify handling of python scripts","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-04-09T15:57:45Z","receivedAt":"2010-04-09T15:57:45Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"The sed script intended to add a standard opening to python scripts\nwas non-compatible and overly complex. Simplifying it down to a set\nof one-liners removes the compatibility issues of newlines. Moving\nthe environment alterations from the Makefile to the python scripts\nmakes also makes the scripts easier to run in-place.\n\nSpecifically, the new sed script:\n\n - Alters the shebang line to use the configured Python.\n - Alters any os.getenv(\"GITPYTHONLIB\") calls to use @@INSTLIBDIR@@ as the\n default. This will replace any existing default or add a default if\n none is provided.\n - Replaces the @@INSTLIBDIR@@ placeholder with the directory git installs\n its python libraries to.\n\nThe last two steps could be combined into a single step, but is left\nseparate in case someone has another need for @@INSTLIBDIR@@ in their\nscript.\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Brian Gernhardt \n---\n\n On Apr 9, 2010, at 11:39 AM, Sverre Rabbelier wrote:\n\n > My sed foo is not that great, can you explain (in the commit message)\n > what exactly the new sed script does?\n \n Certainly. (And the commit message is the only change from v1.)\n\n Makefile | 9 ++-------\n git-remote-testgit.py | 2 ++\n 2 files changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex f0fe351..87c90d6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1629,13 +1629,8 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py\n \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \\\n \t\t--no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' \\\n \t\tinstlibdir` && \\\n-\tsed -e '1{' \\\n-\t -e '\ts|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n-\t -e '}' \\\n-\t -e 's|^import sys.*|&; \\\\\\\n-\t import os; \\\\\\\n-\t sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\\\n-\t \"@@INSTLIBDIR@@\"));|' \\\n+\tsed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n+\t -e 's|\\(os\\.getenv(\"GITPYTHONLIB\"\\)[^)]*)|\\1,\"@@INSTLIBDIR@@\")|' \\\n \t -e 's|@@INSTLIBDIR@@|'\"$$INSTLIBDIR\"'|g' \\\n \t $@.py >$@+ && \\\n \tchmod +x $@+ && \\\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex f61624e..9253922 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -2,6 +2,8 @@\n \n import hashlib\n import sys\n+import os\n+sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\".\"))\n \n from git_remote_helpers.util import die, debug, warn\n from git_remote_helpers.git.repo import GitRepo\n-- \n1.7.1.rc0.243.g2ce66\n"},{"id":"139052","messageId":"n2qfabb9a1e1004090905u7a28136bzc1bf556f1d6149e0@mail.gmail.com","threadId":"23392","inReplyTo":"1270828665-29373-1-git-send-email-brian@gernhardtsoftware.com","subject":"Re: [PATCH v2] Makefile: Simplify handling of python scripts","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-04-09T16:05:43Z","receivedAt":"2010-04-09T16:05:43Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":" Heya,\n\nOn Fri, Apr 9, 2010 at 17:57, Brian Gernhardt\n wrote:\n>  Certainly.  (And the commit message is the only change from v1.)\n\nThanks, very clear!\n\nAcked-by: Sverre Rabbelier \n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"139055","messageId":"201004091831.49066.jnareb@gmail.com","threadId":"23392","inReplyTo":"1270827746-29229-1-git-send-email-brian@gernhardtsoftware.com","subject":"Re: [PATCH v2 1/3] send-email: Don't use FQDNs without a '.'","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-04-09T16:31:48Z","receivedAt":"2010-04-09T16:31:48Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, 9 Apr 2010, Brian Gernhardt wrote:\n \n> Since the same condition is used twice and getting complex, let's move\n> it to a new function.\n\nGood idea.\n\n\nNote that the comments below are just nitpicking about Perl style.\n \n> @@ -863,14 +863,19 @@ sub sanitize_address\n> # This maildomain*() code is based on ideas in Perl library Test::Reporter\n> # /usr/share/perl5/Test/Reporter/Mail/Util.pm ==> sub _maildomain ()\n> \n> +sub valid_fqdn\n> +{\n> +\tmy $domain = $_[0];\n> +\treturn !($^O eq 'darwin' && $domain =~ /\\.local$/) && $domain =~ /\\./;\n> +}\n\nA matter of style: in Perl it more usual to use\n\n sub {\n \t...\n }\n\nstyle rather than\n\n sub \n {\n \t...\n }\n\nUnfortunately git-send-email.perl is a bit inconsistent in the style used;\n23 subroutines use Perl style, 5 subroutines including previous one i.e.\nsanitize_address use C-like style (one of).\n\n\nAlso, the usual way of unrolling @_; is to use either\n\n my ($par1, $par2, ...) = @_;\n\nor use\n\n mu $par = shift;\n\nThe form $_[0] etc. is used very rarely. I think it is even against \nPerl Best Practices (see http://www.perlcritic.org and Perl::Critic).\n\n\nSo in my opinion this fragment should be:\n\n+sub valid_fqdn {\n+\tmy $domain = shift;\n+\treturn !($^O eq 'darwin' && $domain =~ /\\.local$/) && $domain =~ /\\./;\n+}\n\n\n> +\n> sub maildomain_net\n> {\n> \tmy $maildomain;\n> \n> \tif (eval { require Net::Domain; 1 }) {\n> \t\tmy $domain = Net::Domain::domainname();\n> -\t\t$maildomain = $domain\n> -\t\t\tunless $^O eq 'darwin' && $domain =~ /\\.local$/;\n> +\t\t$maildomain = $domain if valid_fqdn( $domain );\n> \t}\n> \n> \treturn $maildomain;\n> @@ -887,8 +892,7 @@ sub maildomain_mta\n> \t\t\t\tmy $domain = $smtp->domain;\n> \t\t\t\t$smtp->quit;\n> \n> -\t\t\t\t$maildomain = $domain\n> -\t\t\t\t\tunless $^O eq 'darwin' && $domain =~ /\\.local$/;\n> +\t\t\t\t$maildomain = $domain if valid_fqdn( $domain );\n> \n> \t\t\t\tlast if $maildomain;\n> \t\t\t}\n\nStyle: usually there is no space around function arguments, so \n'valid_fqdn($domain);'.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"139058","messageId":"201004092040.50142.jnareb@gmail.com","threadId":"23392","inReplyTo":"1270827245-28958-4-git-send-email-brian@gernhardtsoftware.com","subject":"Re: [PATCH v2 3/3] send-email: Add sendemail.smtpdomain","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-04-09T18:40:49Z","receivedAt":"2010-04-09T18:40:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, 9 Apr 2010, Brian Gernhardt wrote:\n\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index f491d44..bdfe3f2 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -132,8 +132,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };\n> my $have_mail_address = eval { require Mail::Address; 1 };\n> my $smtp;\n> my $auth;\n> -my $mail_domain_default = \"localhost.localdomain\";\n> -my $mail_domain;\n> +my $smtp_domain_default = \"localhost.localdomain\";\n\nWhy this change, this renaming of variables from $mail_domain_default\nand $mail_domain to $smtp_domain_default and $smtp_domain? Why you\nhave removed this forward declaration of $smtp_domain/$mail_domain?\n \n> sub unique_email_list(@);\n> sub cleanup_compose_files();\n> @@ -190,7 +189,7 @@ sub do_edit {\n> # Variables with corresponding config settings\n> my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);\n> my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);\n> -my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);\n> +my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts, $smtp_domain);\n> my ($validate, $confirm);\n> my (@suppress_cc);\n\nWhy have you moved $smtp_domain declaration (formerly $mail_domain)\nhere? And why it is not described in commit message (at least \"Cleanup.\",\nor something like this)?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"139151","messageId":"CD79B09D-D728-4A3A-945F-9D117C15FFB5@gernhardtsoftware.com","threadId":"23392","inReplyTo":"201004091831.49066.jnareb@gmail.com","subject":"Re: [PATCH v2 1/3] send-email: Don't use FQDNs without a '.'","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-04-10T13:44:24Z","receivedAt":"2010-04-10T13:44:24Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"\nOn Apr 9, 2010, at 12:31 PM, Jakub Narebski wrote:\n\n> Note that the comments below are just nitpicking about Perl style.\n\nFair enough. I've been using Ruby and Shell far more than Perl recently. I've gotten a bit rusty.\n\n> A matter of style: in Perl it more usual to use\n> \n> sub {\n> \t...\n> }\n> \n> style rather than\n> \n> sub \n> {\n> \t...\n> }\n> \n> Unfortunately git-send-email.perl is a bit inconsistent in the style used;\n> 23 subroutines use Perl style, 5 subroutines including previous one i.e.\n> sanitize_address use C-like style (one of).\n\nI 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.\n\n> Also, the usual way of unrolling @_; is to use either\n> \n> my ($par1, $par2, ...) = @_;\n> \n> or use\n> \n> mu $par = shift;\n> \n> The form $_[0] etc. is used very rarely. I think it is even against \n> Perl Best Practices (see http://www.perlcritic.org and Perl::Critic).\n\nI knew that. I really did. But I started off trying to write\n\nsub valid_fqdn( $domain )\n\nWhich would be valid Perl 6, but not Perl 5. So then I tried using\n\nmy $domain = $1\n\nWhich, while valid, is wrong. So I changed it to @_[1], @_[0], and finally $_[0]. My brain wasn't running at 100% yesterday, apparently.\n\n> Style: usually there is no space around function arguments, so \n> 'valid_fqdn($domain);'.\n\nUniversity training is difficult to overcome. They demanded spaces nearly everywhere, so I type them by something akin to reflex.\n\nThank you for all the review!\n~~ Brian Gernhardt\n"},{"id":"139152","messageId":"503EF023-77C0-4763-947C-385C67D3875A@gernhardtsoftware.com","threadId":"23392","inReplyTo":"201004092040.50142.jnareb@gmail.com","subject":"Re: [PATCH v2 3/3] send-email: Add sendemail.smtpdomain","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-04-10T13:51:36Z","receivedAt":"2010-04-10T13:51:36Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"\nOn Apr 9, 2010, at 2:40 PM, Jakub Narebski wrote:\n\n> On Fri, 9 Apr 2010, Brian Gernhardt wrote:\n> \n>> diff --git a/git-send-email.perl b/git-send-email.perl\n>> index f491d44..bdfe3f2 100755\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -132,8 +132,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };\n>> my $have_mail_address = eval { require Mail::Address; 1 };\n>> my $smtp;\n>> my $auth;\n>> -my $mail_domain_default = \"localhost.localdomain\";\n>> -my $mail_domain;\n>> +my $smtp_domain_default = \"localhost.localdomain\";\n> \n> Why this change, this renaming of variables from $mail_domain_default\n> and $mail_domain to $smtp_domain_default and $smtp_domain? Why you\n> have removed this forward declaration of $smtp_domain/$mail_domain?\n\nI 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).\n\n>> sub unique_email_list(@);\n>> sub cleanup_compose_files();\n>> @@ -190,7 +189,7 @@ sub do_edit {\n>> # Variables with corresponding config settings\n>> my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);\n>> my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);\n>> -my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);\n>> +my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts, $smtp_domain);\n>> my ($validate, $confirm);\n>> my (@suppress_cc);\n> \n> Why have you moved $smtp_domain declaration (formerly $mail_domain)\n> here? And why it is not described in commit message (at least \"Cleanup.\",\n> or something like this)?\n\n$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.\n\nYes, 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\".\n\nI appreciate the review and help, although I wish I wasn't making a fool of myself in public. ;-)\n\n~~ Brian Gernhardt"}]}