Skip to content

Conversation

@setmao
Copy link
Member

@setmao setmao commented May 17, 2020

Let we can add attachment file when build mail.

Steps to Test This PR

The following steps are also performed and verified.

Send The Rendered Emails with Attachment File

$python3 send_mail.py example/mail_config.json --attachment_file ~/Desktop/社群軌主題及場地.pdf
You are about to send the mails under "mails_to_sent". Do you want to continue? [y/N]: y      
Please enter your mail account: set840526@gmail.com
Please enter you mail password:
INFO:root:Email sent to example@gmail.com!

Expected Result

The receivers could get the email with attachment file, and the file could open/download correctly.

@setmao setmao changed the title Pr attachment file Add attachment file May 17, 2020
@tai271828
Copy link
Member

The feature is awesome and it will be useful to the sponsorship team definitely. There are two items still needing your help:

  1. This pull request is very likely to raise change conflict after this pull request lands Add suffix option #4 You may need to resolve the conflict later.
  2. May you please elaborate how to test your pull request? You may refer to this pull request as an example Add suffix option #4 (see "Steps to Test This PR")

Copy link
Member

@tai271828 tai271828 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me. We just need further information and action described in this comment #5 (comment)

@setmao
Copy link
Member Author

setmao commented May 17, 2020

Thx @tai271828
I update more information for how to test this PR in first comment.

Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, it looks great! Thanks for contributing! I leave 2 comments for minor issues.

@setmao
Copy link
Member Author

setmao commented May 19, 2020

@Lee-W Thx for your comment
I adding type annotation to attachment_file and use if attchement: in latest two commit.

@Lee-W
Copy link
Member

Lee-W commented May 19, 2020

The code looks good to me. Thanks for the contribution! The github action looks a bit weird now. I'll reword some of your commits and handle it on my side

@Lee-W Lee-W force-pushed the pr-attachment-file branch from 0810ff7 to 3039f64 Compare May 19, 2020 02:57
@Lee-W Lee-W merged commit 5968f57 into pycontw:master May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants