[MERGE] Patch: smtp_connection.py

[Martin Pool]

2009/5/7 Aaron Bentley <aaron at aaronbentley.com>:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Amit Saha wrote:
>>> Please submit a patch that makes only the changes you want.
>>
>> Please find the patch attached. Sorry for the earlier trouble.

Thanks for the patch.

I'll just expand a bit on Aaron's correct comments because Amit may be
new to Python:

>>
>>  from bzrlib import (
>>      config,
>> @@ -56,6 +57,11 @@
>>          self._smtp_username = config.get_user_option('smtp_username')
>>          self._smtp_password = config.get_user_option('smtp_password')
>>
>> +      """ If the password is not found in the configuration file, then prompt the user"""
>
> Please use # for comments, not docstrings.

Python has this kind of weird syntax where if the first statement in a
scope is a literal string, it's attached to the __doc__ attribute of
the method/class/module/function/etc and can be shown by help().
However, within a function this does nothing and may waste time.


>
>> +
>> +      if self._smtp_password is "":
>
> Please compare strings with ==, not is.

'is' compares for pointer equality and may not be true for equal
strings.  (Like = vs .equals in java iirc.)

-- 
Martin <http://launchpad.net/~mbp/>