[PATCH] Add SMTP TLS email support to allow gmail use

[dinkypumpkin]

On 26/07/2012 12:44, Bill Denton wrote:
> Add SMTP TLS to support gmail. This allows me to send get_iplayer
> output to myself using gmail as I don't have an email server on the
> Ubuntu Linux machine. Been using for a few months. Patch is against
> current version in git infradead.org. Requires Perl Net::SMTP::TLS,
> which under Ubuntu can be installed by "apt-get install
> libnet-smtp-tls-perl" as root.

This looks like it could be useful, but there are a few problems. 
Basically, your integration of Net::SMTP:SSL appears to be broken:

1. You omitted the port number in the constructor for non-TLS cases. 
Net::SMTP::SSL doesn't define a default port, so you need to pass it to 
the constructor.

2. You omitted a call to the auth() method.  Net::SMTP::SSL doesn't 
perform the full login process in the constructor like Net::SMTP::TLS. 
You just need to invoke auth() for non-TLS cases when a username is 
specified, otherwise skip it.

3. You need to retain the existing error handling when populating 
@mail_failure for non-TLS cases.  Those Net::SMTP methods, and by 
extension the same ones in Net::SMTP::SSL, don't throw an exception on 
error but rather return true/false.

If you decide to have another go at this, here are some general 
suggestions (for anyone) when submitting a patch:

1. Check your patch before sending to the list.  Send it to yourself and 
ensure it hasn't been altered in transit.  You would have seen that some 
extra line breaks had been introduced in the code, so this patch 
couldn't be applied as is.  I'll hazard a guess that you may have 
pasted your patch into the Gmail web interface, which is a known offender.

2. Be Git friendly.  get_iplayer code is managed with Git, so you want 
someone (most probably me) to be able to save your patch email and apply 
it to the source tree with without any extra fiddling.   If you can't 
use Git yourself, then at least use "diff -Naur" on your old/new source 
directories to produce your patch.  That will produce a diff in unified 
format with context (best for Git) rather than the context-less 
CVS-style format used here.  It's a bit more email-friendly as well. 
Look at other patches in the mailing list archive to see the difference.

3. Watch your whitespace.  It is a useful courtesy to conform to the 
formatting conventions already used in the code.  In the eternal 
struggle between tabs and spaces for indentation supremacy, get_iplayer 
is on the side of the tabs.  However, this code somehow ended up with 
mixed tabs and spaces in a couple of spots.

4. Check your spelling.  If you add command-line options, check the 
spelling in the description text.  Remember that the option definitions 
are used to produce the help listing and man page.  There was only a 
swapped pair of letters here ("appropirate"), but it might as well be 
correct.