-
-
Notifications
You must be signed in to change notification settings - Fork 19
fix python3 warning and make it supports smtp2go smtp server #7
base: master
Are you sure you want to change the base?
Conversation
@@ -91,7 +91,7 @@ def read_config(self): | |||
'''Parse the config file. Create a new one if needed.''' | |||
|
|||
self.create_config() | |||
config = configparser.SafeConfigParser() | |||
config = configparser.ConfigParser() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got a warning of "figParser class has been renamed to ConfigParser in Python 3.2. This alias will be removed in future versions. Use ConfigParser directly instead."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, you should add this information to the commit message next time, and it's going to be clear.
@@ -173,7 +173,7 @@ def send_mail(self): | |||
mail_server = smtplib.SMTP(host=self.smtp_server, port=self.smtp_port) | |||
mail_server.starttls() | |||
else: | |||
mail_server = smtplib.SMTP_SSL(host=self.smtp_server, port=self.smtp_port) | |||
mail_server = smtplib.SMTP(host=self.smtp_server, port=self.smtp_port) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. Instead of disabling SSL for everyone, add a new command line option --no-ssl
that will populate self.ssl
value accordingly. This will allow users to disable SSL if they wish. Use it in the if
clause above. Instead of if self.smtp_port == 587:
use something like if not self.ssl:
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. I may add a --no-ssl option later and create another pull request.
Thank you @kparal :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's much better to update this pull request instead of creating a new one. Simply adjust the code in your branch and this PR will get automatically updated.
No description provided.